From 7be6b44f26f2305ff9af961fb79de43c0965c659 Mon Sep 17 00:00:00 2001 From: Raymond Luong Date: Wed, 22 Oct 2025 15:55:41 -0600 Subject: [PATCH 1/2] SF-3601 Navigate to the first chapter of a draft when formatting draft --- .../draft-handling.service.ts | 2 +- .../draft-usfm-format.component.html | 3 +- .../draft-usfm-format.component.scss | 3 + .../draft-usfm-format.component.spec.ts | 96 ++++++++++++++++--- .../draft-usfm-format.component.ts | 25 +++-- .../editor-draft.component.spec.ts | 36 ++----- .../editor-draft/editor-draft.component.ts | 17 +--- .../src/assets/i18n/non_checking_en.json | 4 +- 8 files changed, 122 insertions(+), 64 deletions(-) diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-handling.service.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-handling.service.ts index f6a44a97f61..30e831a9fa6 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-handling.service.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-handling.service.ts @@ -280,7 +280,7 @@ export class DraftHandlingService { */ opsHaveContent(ops: DeltaOperation[]): boolean { const indexOfFirstText = ops.findIndex(op => typeof op.insert === 'string'); - const onlyTextOpIsTrailingNewline = indexOfFirstText === ops.length - 1 && ops[indexOfFirstText].insert === '\n'; + const onlyTextOpIsTrailingNewline = indexOfFirstText === ops.length - 1 && ops[indexOfFirstText]?.insert === '\n'; const hasNoExistingText = indexOfFirstText === -1 || onlyTextOpIsTrailingNewline; return !hasNoExistingText; } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.html b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.html index 8a1820a9be8..42066db46ce 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.html +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.html @@ -75,7 +75,7 @@

{{ t("formatting_options") }}

[book]="bookNum" (bookChange)="bookChanged($event)" [(chapter)]="chapterNum" - [chapters]="chapters" + [chapters]="chaptersWithDrafts" (chapterChange)="chapterChanged($event)" >
@@ -83,6 +83,7 @@

{{ t("formatting_options") }}

[isReadOnly]="true" [subscribeToUpdates]="false" [isRightToLeft]="isRightToLeft" + [placeholder]="isOnline ? t('chapter_draft_does_not_exist') : t('text_unavailable_offline')" [class.initializing]="isInitializing" [class.loading]="isLoadingData" [style.--project-font]="projectFont" diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.scss b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.scss index 5c666b1aeeb..47ed4bb459d 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.scss +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.scss @@ -13,6 +13,7 @@ flex-direction: row; column-gap: 8px; height: 100%; + width: 100%; margin-top: 8px; @include breakpoints.media-breakpoint-down(sm) { @@ -48,6 +49,7 @@ display: flex; flex-direction: column; overflow: hidden; + width: 100%; @include breakpoints.media-breakpoint-down(sm) { height: 600px; @@ -55,6 +57,7 @@ } .viewer-container { + height: 100%; overflow-y: auto; } } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.spec.ts index 3f2e560dd75..bf22647c691 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.spec.ts @@ -4,12 +4,15 @@ import { ComponentFixture, fakeAsync, TestBed, tick } from '@angular/core/testin import { MatRadioButtonHarness } from '@angular/material/radio/testing'; import { provideNoopAnimations } from '@angular/platform-browser/animations'; import { ActivatedRoute } from '@angular/router'; +import { SFProjectProfile } from 'realtime-server/lib/esm/scriptureforge/models/sf-project'; import { createTestProjectProfile } from 'realtime-server/lib/esm/scriptureforge/models/sf-project-test-data'; import { TextInfo } from 'realtime-server/lib/esm/scriptureforge/models/text-info'; import { + DraftConfig, DraftUsfmConfig, ParagraphBreakFormat, - QuoteFormat + QuoteFormat, + TranslateConfig } from 'realtime-server/lib/esm/scriptureforge/models/translate-config'; import { of } from 'rxjs'; import { anything, deepEqual, mock, verify, when } from 'ts-mockito'; @@ -78,8 +81,15 @@ describe('DraftUsfmFormatComponent', () => { it('shows message if user is not online', fakeAsync(async () => { const env = new TestEnvironment({ - config: { paragraphFormat: ParagraphBreakFormat.MoveToEnd, quoteFormat: QuoteFormat.Denormalized } + project: { + translateConfig: { + draftConfig: { + usfmConfig: { paragraphFormat: ParagraphBreakFormat.MoveToEnd, quoteFormat: QuoteFormat.Normalized } + } as DraftConfig + } as TranslateConfig + } }); + expect(env.offlineMessage).toBeNull(); env.onlineStatusService.setIsOnline(false); @@ -102,19 +112,63 @@ describe('DraftUsfmFormatComponent', () => { verify(mockedDraftHandlingService.getDraft(anything(), anything())).once(); })); + it('can navigate to first book and chapter if book does not exist', fakeAsync(() => { + when(mockedActivatedRoute.params).thenReturn(of({ bookId: 'NUM', chapter: '1' })); + const env = new TestEnvironment(); + tick(EDITOR_READY_TIMEOUT); + env.fixture.detectChanges(); + tick(EDITOR_READY_TIMEOUT); + expect(env.component.bookNum).toBe(1); + expect(env.component.chapterNum).toBe(1); + verify(mockedDraftHandlingService.getDraft(anything(), anything())).once(); + })); + + it('can navigate to book and chapter if first chapter has no draft', fakeAsync(() => { + const env = new TestEnvironment({ + project: { + texts: [ + { + bookNum: 1, + chapters: [ + { number: 1, lastVerse: 15, isValid: true, permissions: {}, hasDraft: false }, + { number: 2, lastVerse: 20, isValid: true, permissions: {}, hasDraft: true }, + { number: 3, lastVerse: 18, isValid: true, permissions: {}, hasDraft: true } + ], + hasSource: true, + permissions: {} + } + ] + } + }); + tick(EDITOR_READY_TIMEOUT); + env.fixture.detectChanges(); + tick(EDITOR_READY_TIMEOUT); + expect(env.component.bookNum).toBe(1); + expect(env.component.chapterNum).toBe(2); + expect(env.component.chaptersWithDrafts).toEqual([2, 3]); + verify(mockedDraftHandlingService.getDraft(anything(), anything())).once(); + })); + // Book and chapter changed it('navigates to a different book and chapter', fakeAsync(() => { const env = new TestEnvironment({ - config: { paragraphFormat: ParagraphBreakFormat.MoveToEnd, quoteFormat: QuoteFormat.Denormalized } + project: { + translateConfig: { + draftConfig: { + usfmConfig: { paragraphFormat: ParagraphBreakFormat.MoveToEnd, quoteFormat: QuoteFormat.Denormalized } + } as DraftConfig + } as TranslateConfig + } }); + verify(mockedDraftHandlingService.getDraft(anything(), anything())).once(); - expect(env.component.chapters.length).toEqual(1); + expect(env.component.chaptersWithDrafts.length).toEqual(1); expect(env.component.booksWithDrafts.length).toEqual(2); env.component.bookChanged(2); tick(); env.fixture.detectChanges(); - expect(env.component.chapters.length).toEqual(2); + expect(env.component.chaptersWithDrafts.length).toEqual(2); verify(mockedDraftHandlingService.getDraft(anything(), anything())).twice(); env.component.chapterChanged(2); @@ -133,7 +187,13 @@ describe('DraftUsfmFormatComponent', () => { it('should show the currently selected format options', fakeAsync(() => { const env = new TestEnvironment({ - config: { paragraphFormat: ParagraphBreakFormat.MoveToEnd, quoteFormat: QuoteFormat.Normalized } + project: { + translateConfig: { + draftConfig: { + usfmConfig: { paragraphFormat: ParagraphBreakFormat.MoveToEnd, quoteFormat: QuoteFormat.Normalized } + } as DraftConfig + } as TranslateConfig + } }); expect(env.component.paragraphFormat.value).toBe(ParagraphBreakFormat.MoveToEnd); expect(env.component.quoteFormat.value).toBe(QuoteFormat.Normalized); @@ -141,7 +201,13 @@ describe('DraftUsfmFormatComponent', () => { it('goes back if user chooses different configurations and then goes back', fakeAsync(async () => { const env = new TestEnvironment({ - config: { paragraphFormat: ParagraphBreakFormat.MoveToEnd, quoteFormat: QuoteFormat.Denormalized } + project: { + translateConfig: { + draftConfig: { + usfmConfig: { paragraphFormat: ParagraphBreakFormat.MoveToEnd, quoteFormat: QuoteFormat.Denormalized } + } as DraftConfig + } as TranslateConfig + } }); verify(mockedDraftHandlingService.getDraft(anything(), anything())).once(); expect(env.harnesses?.length).toEqual(5); @@ -162,7 +228,13 @@ describe('DraftUsfmFormatComponent', () => { it('should save changes to the draft format', fakeAsync(async () => { const env = new TestEnvironment({ - config: { paragraphFormat: ParagraphBreakFormat.MoveToEnd, quoteFormat: QuoteFormat.Denormalized } + project: { + translateConfig: { + draftConfig: { + usfmConfig: { paragraphFormat: ParagraphBreakFormat.MoveToEnd, quoteFormat: QuoteFormat.Denormalized } + } as DraftConfig + } as TranslateConfig + } }); verify(mockedDraftHandlingService.getDraft(anything(), anything())).once(); expect(env.harnesses?.length).toEqual(5); @@ -212,7 +284,7 @@ class TestEnvironment { readonly projectId = 'project01'; onlineStatusService: TestOnlineStatusService; - constructor(args: { config?: DraftUsfmConfig; quotationAnalysis?: QuotationAnalysis } = {}) { + constructor(args: { project?: Partial; quotationAnalysis?: QuotationAnalysis } = {}) { const userDoc = mock(UserDoc); this.onlineStatusService = TestBed.inject(OnlineStatusService) as TestOnlineStatusService; when(mockedDraftGenerationService.getLastCompletedBuild(anything())).thenReturn( @@ -232,7 +304,7 @@ class TestEnvironment { when(mockedNoticeService.show(anything())).thenResolve(); when(mockedDialogService.confirm(anything(), anything(), anything())).thenResolve(true); when(mockedServalAdministration.onlineRetrievePreTranslationStatus(anything())).thenResolve(); - this.setupProject(args.config); + this.setupProject(args.project); this.fixture = TestBed.createComponent(DraftUsfmFormatComponent); this.component = this.fixture.componentInstance; const loader = TestbedHarnessEnvironment.loader(this.fixture); @@ -257,7 +329,7 @@ class TestEnvironment { return this.fixture.nativeElement.querySelector('.quote-format-warning'); } - setupProject(config?: DraftUsfmConfig): void { + setupProject(project?: Partial): void { const texts: TextInfo[] = [ { bookNum: 1, @@ -283,7 +355,7 @@ class TestEnvironment { ]; const projectDoc = { id: this.projectId, - data: createTestProjectProfile({ translateConfig: { draftConfig: { usfmConfig: config } }, texts }) + data: createTestProjectProfile({ translateConfig: project?.translateConfig, texts: project?.texts ?? texts }) } as SFProjectProfileDoc; when(mockedActivatedProjectService.projectId).thenReturn(this.projectId); when(mockedActivatedProjectService.projectDoc$).thenReturn(of(projectDoc)); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.ts index 4d06bf530d4..0861d2fa5db 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-usfm-format/draft-usfm-format.component.ts @@ -10,6 +10,7 @@ import { ActivatedRoute } from '@angular/router'; import { TranslocoModule } from '@ngneat/transloco'; import { Canon } from '@sillsdev/scripture'; import { Delta } from 'quill'; +import { SFProjectProfile } from 'realtime-server/lib/esm/scriptureforge/models/sf-project'; import { TextInfo } from 'realtime-server/lib/esm/scriptureforge/models/text-info'; import { DraftUsfmConfig, @@ -63,7 +64,7 @@ export class DraftUsfmFormatComponent extends DataLoadingComponent implements Af bookNum: number = 1; booksWithDrafts: number[] = []; chapterNum: number = 1; - chapters: number[] = []; + chaptersWithDrafts: number[] = []; isInitializing: boolean = true; paragraphBreakFormat = ParagraphBreakFormat; quoteStyle = QuoteFormat; @@ -156,11 +157,11 @@ export class DraftUsfmFormatComponent extends DataLoadingComponent implements Af defaultBook = Canon.bookIdToNumber(params['bookId']); } let defaultChapter = 1; - this.chapters = texts.find(t => t.bookNum === defaultBook)?.chapters.map(c => c.number) ?? []; - if (params['chapter'] !== undefined && this.chapters.includes(Number(params['chapter']))) { + this.chaptersWithDrafts = this.getChaptersWithDrafts(defaultBook, projectDoc.data); + if (params['chapter'] !== undefined && this.chaptersWithDrafts.includes(Number(params['chapter']))) { defaultChapter = Number(params['chapter']); - } else if (this.chapters.length > 0) { - defaultChapter = this.chapters[0]; + } else if (this.chaptersWithDrafts.length > 0) { + defaultChapter = this.chaptersWithDrafts[0]; } this.projectFont = this.fontService.getFontFamilyFromProject(projectDoc); this.bookChanged(defaultBook, defaultChapter); @@ -190,9 +191,8 @@ export class DraftUsfmFormatComponent extends DataLoadingComponent implements Af bookChanged(bookNum: number, chapterNum?: number): void { this.bookNum = bookNum; - const texts = this.activatedProjectService.projectDoc!.data!.texts; - this.chapters = texts.find(t => t.bookNum === this.bookNum)?.chapters.map(c => c.number) ?? []; - this.chapterNum = chapterNum ?? this.chapters[0] ?? 1; + this.chaptersWithDrafts = this.getChaptersWithDrafts(bookNum, this.activatedProjectService.projectDoc!.data!); + this.chapterNum = chapterNum ?? this.chaptersWithDrafts[0] ?? 1; this.reloadText(); } @@ -258,4 +258,13 @@ export class DraftUsfmFormatComponent extends DataLoadingComponent implements Af if (isOnline) this.reloadText(); }); } + + private getChaptersWithDrafts(bookNum: number, project: SFProjectProfile): number[] { + return ( + project.texts + .find(t => t.bookNum === bookNum) + ?.chapters.filter(c => !!c.hasDraft && c.lastVerse > 0) + .map(c => c.number) ?? [] + ); + } } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.spec.ts index 21a50dab81f..ac4308d1b43 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.spec.ts @@ -86,6 +86,7 @@ describe('EditorDraftComponent', () => { buildProgress$.next({ state: BuildStates.Completed } as BuildDto); when(mockActivatedProjectService.projectId$).thenReturn(of('targetProjectId')); when(mockDraftGenerationService.getLastCompletedBuild(anything())).thenReturn(of(undefined)); + when(mockDraftHandlingService.opsHaveContent(anything())).thenReturn(true); fixture = TestBed.createComponent(EditorDraftComponent); component = fixture.componentInstance; @@ -308,6 +309,7 @@ describe('EditorDraftComponent', () => { when(mockDraftHandlingService.getDraft(anything(), anything())).thenReturn(of(cloneDeep(emptyDraftDelta.ops!))); when(mockDraftHandlingService.draftDataToOps(anything(), anything())).thenReturn(emptyDraftDelta.ops!); when(mockDraftHandlingService.isDraftSegmentMap(anything())).thenReturn(false); + when(mockDraftHandlingService.opsHaveContent(anything())).thenReturn(false); // SUT fixture.detectChanges(); @@ -319,35 +321,12 @@ describe('EditorDraftComponent', () => { flush(); })); - it('should show draft empty if earlier draft exists but history is not enabled', fakeAsync(() => { + it('should set editor to empty state when no revision', fakeAsync(() => { const testProjectDoc: SFProjectProfileDoc = { data: createTestProjectProfile() } as SFProjectProfileDoc; - when(mockFeatureFlagService.newDraftHistory).thenReturn(createTestFeatureFlag(false)); when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(false)); - when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( - of(draftHistory) - ); - when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc)); - spyOn(component, 'getTargetOps').and.returnValue(of(targetDelta.ops!)); - - fixture.detectChanges(); - tick(EDITOR_READY_TIMEOUT); - - verify(mockDraftHandlingService.getDraft(anything(), anything())).never(); - verify(mockDraftHandlingService.draftDataToOps(anything(), anything())).never(); - expect(component.draftCheckState).toEqual('draft-empty'); - flush(); - })); - - it('should return ops and update the editor when no revision', fakeAsync(() => { - const testProjectDoc: SFProjectProfileDoc = { - data: createTestProjectProfile() - } as SFProjectProfileDoc; - when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); - when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( - of(undefined) - ); + when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn(of([])); when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc)); spyOn(component, 'getTargetOps').and.returnValue(of(targetDelta.ops!)); when(mockDraftHandlingService.getDraft(anything(), anything())).thenReturn(of(cloneDeep(draftDelta.ops!))); @@ -357,10 +336,9 @@ describe('EditorDraftComponent', () => { fixture.detectChanges(); tick(EDITOR_READY_TIMEOUT); - verify(mockDraftHandlingService.getDraft(anything(), anything())).once(); - verify(mockDraftHandlingService.draftDataToOps(anything(), anything())).once(); - expect(component.draftCheckState).toEqual('draft-present'); - expect(component.draftText.editor!.getContents().ops).toEqual(draftDelta.ops); + verify(mockDraftHandlingService.getDraft(anything(), anything())).never(); + verify(mockDraftHandlingService.draftDataToOps(anything(), anything())).never(); + expect(component.draftCheckState).toEqual('draft-empty'); flush(); })); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts index 4b95d52362f..d91fd8d753e 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts @@ -206,7 +206,7 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { ) .pipe( map(revisions => { - if (revisions != null) { + if (revisions != null && revisions.length > 0) { // Sort revisions by timestamp descending this.draftRevisions = [...revisions].sort((a, b) => { return new Date(b.timestamp).getTime() - new Date(a.timestamp).getTime(); @@ -229,9 +229,8 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { ]) ), switchMap(([timestamp, draftExists]) => { - const initialTimestamp: Date | undefined = timestamp ?? this.timestamp; // If an earlier draft exists, hide it if the draft history feature is not enabled - if (!draftExists && (!this.featureFlags.newDraftHistory.enabled || initialTimestamp == null)) { + if (!draftExists && timestamp == null) { this.draftCheckState = 'draft-empty'; return EMPTY; } @@ -243,7 +242,7 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { this.targetProject = projectDoc.data; }), distinctUntilChanged(), - map(() => initialTimestamp) + map(() => timestamp) ); }), switchMap((timestamp: Date | undefined) => @@ -266,14 +265,8 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { }), switchMap(({ targetOps, draftOps }) => { // Look for verses that contain text. If these are present, this is a non-empty draft - for (const op of draftOps) { - if (op.attributes != null && op.attributes.segment != null && typeof op.insert === 'string') { - const hasText: boolean = op.insert.trim().length > 0; - const segRef: string = op.attributes.segment as string; - if (hasText && segRef.startsWith('verse_')) { - return of({ targetOps, draftOps }); - } - } + if (this.draftHandlingService.opsHaveContent(draftOps)) { + return of({ targetOps, draftOps }); } // No generated verse segments were found, return an empty draft diff --git a/src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json b/src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json index f57a7616ff5..e021ae3adeb 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json +++ b/src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json @@ -357,6 +357,7 @@ }, "draft_usfm_format": { "cancel": "Cancel", + "chapter_draft_does_not_exist": "The draft for this chapter does not exist", "connect_to_the_internet": "Please connect to the internet to change the draft formatting options.", "draft_format_description": "The draft has been created. Here are some formatting options for the text. You can try each option to see the difference it makes.", "failed_to_save": "Failed to save changes. Try again later.", @@ -377,7 +378,8 @@ "quote_style_straight": "Straight quotes", "quote_style_straight_description": "Use only straight quotes in the draft.", "quote_style_title": "Quote style", - "save_changes": "Save" + "save_changes": "Save", + "text_unavailable_offline": "Please connect to the internet to preview the draft" }, "editor": { "add_comment": "Add Comment", From 5918f9b5f548a4c560eff7f269aa8080408c014a Mon Sep 17 00:00:00 2001 From: Raymond Luong Date: Thu, 30 Oct 2025 11:37:17 -0600 Subject: [PATCH 2/2] Remove unnecessary call for draft exists --- .../editor-draft.component.spec.ts | 14 ---- .../editor-draft/editor-draft.component.ts | 64 ++++++++----------- 2 files changed, 26 insertions(+), 52 deletions(-) diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.spec.ts index ac4308d1b43..2716ee24708 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.spec.ts @@ -114,7 +114,6 @@ describe('EditorDraftComponent', () => { data: createTestProjectProfile() } as SFProjectProfileDoc; when(mockFeatureFlagService.newDraftHistory).thenReturn(createTestFeatureFlag(true)); - when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( of(draftHistory) ); @@ -150,7 +149,6 @@ describe('EditorDraftComponent', () => { when(mockFeatureFlagService.newDraftHistory).thenReturn(createTestFeatureFlag(false)); when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc)); - when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( of(draftHistory) ); @@ -188,7 +186,6 @@ describe('EditorDraftComponent', () => { const testProjectDoc: SFProjectProfileDoc = { data: createTestProjectProfile() } as SFProjectProfileDoc; - when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( of(draftHistory) ); @@ -213,7 +210,6 @@ describe('EditorDraftComponent', () => { data: createTestProjectProfile() } as SFProjectProfileDoc; when(mockFeatureFlagService.newDraftHistory).thenReturn(createTestFeatureFlag(true)); - when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( of(draftHistory) ); @@ -242,7 +238,6 @@ describe('EditorDraftComponent', () => { data: createTestProjectProfile() } as SFProjectProfileDoc; when(mockFeatureFlagService.newDraftHistory).thenReturn(createTestFeatureFlag(true)); - when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( of(draftHistory) ); @@ -274,7 +269,6 @@ describe('EditorDraftComponent', () => { data: createTestProjectProfile() } as SFProjectProfileDoc; when(mockFeatureFlagService.newDraftHistory).thenReturn(createTestFeatureFlag(true)); - when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(false)); when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( of(draftHistory) ); @@ -300,7 +294,6 @@ describe('EditorDraftComponent', () => { data: createTestProjectProfile() } as SFProjectProfileDoc; when(mockFeatureFlagService.newDraftHistory).thenReturn(createTestFeatureFlag(true)); - when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(false)); when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( of(draftHistory) ); @@ -325,7 +318,6 @@ describe('EditorDraftComponent', () => { const testProjectDoc: SFProjectProfileDoc = { data: createTestProjectProfile() } as SFProjectProfileDoc; - when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(false)); when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn(of([])); when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc)); spyOn(component, 'getTargetOps').and.returnValue(of(targetDelta.ops!)); @@ -347,7 +339,6 @@ describe('EditorDraftComponent', () => { data: createTestProjectProfile() } as SFProjectProfileDoc; when(mockFeatureFlagService.newDraftHistory).thenReturn(createTestFeatureFlag(false)); - when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( of(draftHistory) ); @@ -390,7 +381,6 @@ describe('EditorDraftComponent', () => { } }) } as SFProjectProfileDoc; - when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( of(draftHistory) ); @@ -412,7 +402,6 @@ describe('EditorDraftComponent', () => { const testProjectDoc: SFProjectProfileDoc = { data: createTestProjectProfile() } as SFProjectProfileDoc; - when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( of(draftHistory) ); @@ -439,7 +428,6 @@ describe('EditorDraftComponent', () => { const testProjectDoc: SFProjectProfileDoc = { data: createTestProjectProfile() } as SFProjectProfileDoc; - when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( of(draftHistory) ); @@ -472,7 +460,6 @@ describe('EditorDraftComponent', () => { const testProjectDoc: SFProjectProfileDoc = { data: createTestProjectProfile() } as SFProjectProfileDoc; - when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( of(draftHistory) ); @@ -500,7 +487,6 @@ describe('EditorDraftComponent', () => { const testProjectDoc: SFProjectProfileDoc = { data: createTestProjectProfile() } as SFProjectProfileDoc; - when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true)); when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn( of(draftHistory) ); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts index d91fd8d753e..fabcc664e81 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts @@ -196,41 +196,34 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { filter(([isOnline, build]) => isOnline && build != null && build.state !== BuildStates.Finishing), tap(() => this.setInitialState()), switchMap(() => - combineLatest([ - merge( - this.draftGenerationService - .getGeneratedDraftHistory( - this.textDocId!.projectId, - this.textDocId!.bookNum, - this.textDocId!.chapterNum - ) - .pipe( - map(revisions => { - if (revisions != null && revisions.length > 0) { - // Sort revisions by timestamp descending - this.draftRevisions = [...revisions].sort((a, b) => { - return new Date(b.timestamp).getTime() - new Date(a.timestamp).getTime(); - }); - const date = this.timestamp ?? new Date(); - // Don't emit this.selectedRevision$, as the merge will handle this - this.selectedRevision = this.findClosestRevision(date, this.draftRevisions); - return date; - } else { - return undefined; - } - }) - ), - this.selectedRevision$.pipe( - filter((rev): rev is { timestamp: string } => rev != null), - map(revision => new Date(revision.timestamp)) - ) - ), - this.draftExists() - ]) + merge( + this.draftGenerationService + .getGeneratedDraftHistory(this.textDocId!.projectId, this.textDocId!.bookNum, this.textDocId!.chapterNum) + .pipe( + map(revisions => { + if (revisions != null && revisions.length > 0) { + // Sort revisions by timestamp descending + this.draftRevisions = [...revisions].sort((a, b) => { + return new Date(b.timestamp).getTime() - new Date(a.timestamp).getTime(); + }); + const date = this.timestamp ?? new Date(); + // Don't emit this.selectedRevision$, as the merge will handle this + this.selectedRevision = this.findClosestRevision(date, this.draftRevisions); + return date; + } else { + return undefined; + } + }) + ), + this.selectedRevision$.pipe( + filter((rev): rev is { timestamp: string } => rev != null), + map(revision => new Date(revision.timestamp)) + ) + ) ), - switchMap(([timestamp, draftExists]) => { + switchMap(timestamp => { // If an earlier draft exists, hide it if the draft history feature is not enabled - if (!draftExists && timestamp == null) { + if (timestamp == null) { this.draftCheckState = 'draft-empty'; return EMPTY; } @@ -335,11 +328,6 @@ export class EditorDraftComponent implements AfterViewInit, OnChanges { this.userAppliedDraft = false; } - private draftExists(): Observable { - // This method of checking for draft may be temporary until there is a better way supplied by serval - return this.draftGenerationService.draftExists(this.projectId!, this.bookNum!, this.chapter!); - } - private findClosestRevision(date: Date, revisions: Revision[]): Revision | undefined { const targetTime: number = date.getTime();