SF-3602 Require usfmConfig to be set before displaying draft tab

[josephmyers]

This change is 

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.31%. Comparing base (eea0a15) to head (f099ee0).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3496   +/-   ##
=======================================
  Coverage   82.30%   82.31%           
=======================================
  Files         613      614    +1     
  Lines       36877    36883    +6     
  Branches     6046     6024   -22     
=======================================
+ Hits        30352    30359    +7     
- Misses       5629     5642   +13     
+ Partials      896      882   -14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pmachapman]

@Nateowami (as you created the original ticket) Is it correct that this PR is pretty much rolling back the work done it SF-3566 Guide user to formatting options on draft tab (#3447)

@pmachapman reviewed 3 of 4 files at r1, 5 of 5 files at r2, all commit messages.
Reviewable status: 8 of 9 files reviewed, 8 unresolved discussions

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.spec.ts line 4100 at r2 (raw file):

        if (projectDoc.data != null) {
          (projectDoc.data.translateConfig as any).draftConfig = { usfmConfig: {} };
        }

I wonder whether it makes sense to have Project01's initial data to have usfmConfig specified, rather than setting it in these 6 tests, and have the one test that requires usfmConfig to be null (should not add draft preview tab when draft formatting (usfmConfig) is not set) explicitly specify it as null?

Code quote:

        const projectDoc = env.getProjectDoc('project01');
        if (projectDoc.data != null) {
          (projectDoc.data.translateConfig as any).draftConfig = { usfmConfig: {} };
        }

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.html line 78 at r2 (raw file):

            </app-notice>
          }
        </div>

Can you move this </div> back above the notice? It looks kinda funny:

Code quote:

          @if (!canApplyDraft) {
            <app-notice icon="warning" type="warning" mode="fill-dark">
              {{ "editor_draft_tab.cannot_import" | transloco }}
            </app-notice>
          }
        </div>

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.scss line 34 at r2 (raw file):

  display: flex;
  justify-content: flex-end;
  column-gap: 4px;

Curious on why you rolled this back to column-gap - I think gap is better if the screen gets squished?

Code quote:

column-gap: 4px;

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.ts line 1491 at r2 (raw file):

      this.userService.currentUserId
    );
    const hasSetDraftFormatting =

NIT: Can you please specify the type

Code quote:

const hasSetDraftFormatting =

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts line 124 at r2 (raw file):

  get bookId(): string {
    return this.bookNum !== undefined ? Canon.bookNumberToId(this.bookNum) : '';
  }

I think the place this was previously is more logical - can you revert this?

Code quote:

  get bookId(): string {
    return this.bookNum !== undefined ? Canon.bookNumberToId(this.bookNum) : '';
  }

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts line 255 at r2 (raw file):

        ?.hasDraft ?? false
    );
  }

I think the place these were previously is more logical - can you revert this?

Code quote:

  get canApplyDraft(): boolean {
    if (this.targetProject == null || this.bookNum == null || this.chapter == null || this.draftDelta?.ops == null) {
      return false;
    }
    return this.draftHandlingService.canApplyDraft(this.targetProject, this.bookNum, this.chapter, this.draftDelta.ops);
  }

  get doesLatestHaveDraft(): boolean {
    return (
      this.targetProject?.texts.find(t => t.bookNum === this.bookNum)?.chapters.find(c => c.number === this.chapter)
        ?.hasDraft ?? false
    );
  }

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts line 120 at r2 (raw file):

        ?.hasDraft ?? false
    );
  }

i.e. revert these 3 functions back to this position.

Code quote:

  get bookId(): string {
    return this.bookNum !== undefined ? Canon.bookNumberToId(this.bookNum) : '';
  }

  get canApplyDraft(): boolean {
    if (this.targetProject == null || this.bookNum == null || this.chapter == null || this.draftDelta?.ops == null) {
      return false;
    }
    return this.draftHandlingService.canApplyDraft(this.targetProject, this.bookNum, this.chapter, this.draftDelta.ops);
  }

  get doesLatestHaveDraft(): boolean {
    return (
      this.targetProject?.texts.find(t => t.bookNum === this.bookNum)?.chapters.find(c => c.number === this.chapter)
        ?.hasDraft ?? false
    );
  }

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.spec.ts line 427 at r2 (raw file):

      expect(component.mustChooseFormattingOptions).toBe(false);
      flush();
    }));

I don't think this test is obsoleted by your changes?

Code quote:

    it('should allow user to apply draft when formatting selected', fakeAsync(() => {
      const testProjectDoc: SFProjectProfileDoc = {
        data: createTestProjectProfile({
          texts: [
            {
              bookNum: 1,
              chapters: [{ number: 1, permissions: { user01: SFProjectRole.ParatextAdministrator }, hasDraft: true }]
            }
          ],
          translateConfig: {
            draftConfig: {
              usfmConfig: { paragraphFormat: ParagraphBreakFormat.BestGuess, quoteFormat: QuoteFormat.Denormalized }
            }
          }
        })
      } as SFProjectProfileDoc;
      when(mockDraftGenerationService.draftExists(anything(), anything(), anything())).thenReturn(of(true));
      when(mockDraftGenerationService.getGeneratedDraftHistory(anything(), anything(), anything())).thenReturn(
        of(draftHistory)
      );
      when(mockActivatedProjectService.changes$).thenReturn(of(testProjectDoc));
      when(mockDialogService.confirm(anything(), anything())).thenResolve(true);
      spyOn<any>(component, 'getTargetOps').and.returnValue(of(targetDelta.ops));
      when(mockDraftHandlingService.getDraft(anything(), anything())).thenReturn(of(draftDelta.ops!));
      when(mockDraftHandlingService.draftDataToOps(anything(), anything())).thenReturn(draftDelta.ops!);

      fixture.detectChanges();
      tick(EDITOR_READY_TIMEOUT);

      expect(component.mustChooseFormattingOptions).toBe(false);
      flush();
    }));

[pmachapman]

@pmachapman reviewed 2 of 4 files at r1.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @josephmyers)

[josephmyers]

Reviewable status: 5 of 9 files reviewed, 7 unresolved discussions (waiting on @pmachapman)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.spec.ts line 4100 at r2 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

I wonder whether it makes sense to have Project01's initial data to have usfmConfig specified, rather than setting it in these 6 tests, and have the one test that requires usfmConfig to be null (should not add draft preview tab when draft formatting (usfmConfig) is not set) explicitly specify it as null?

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.html line 78 at r2 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

Can you move this </div> back above the notice? It looks kinda funny:

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.scss line 34 at r2 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

Curious on why you rolled this back to column-gap - I think gap is better if the screen gets squished?

Done. I just rolled back Raymond's commit (after checking with him).

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts line 120 at r2 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

i.e. revert these 3 functions back to this position.

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts line 124 at r2 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

I think the place this was previously is more logical - can you revert this?

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.ts line 255 at r2 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

I think the place these were previously is more logical - can you revert this?

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor-draft/editor-draft.component.spec.ts line 427 at r2 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

I don't think this test is obsoleted by your changes?

That's fair. I just had to change the property under test

[Nateowami]

@pmachapman Yes, it's rolling back that change, and instead not allowing the tab to be opened in that state (the reason is because there were still things not handled in the tab correctly, and so it was either double down with more work, or reverse course and make things simpler).

@Nateowami reviewed 1 of 4 files at r1.
Reviewable status: 4 of 9 files reviewed, 7 unresolved discussions (waiting on @pmachapman)

[josephmyers]

I am seeing that changing the default config is breaking a bunch of test elsewhere in the file. I'll have to finish this tomorrow

Reviewable status: 4 of 9 files reviewed, 7 unresolved discussions (waiting on @pmachapman)

[Nateowami]

@josephmyers If changing the tests like that is slowing down the process too much, then let's just leave them as-is. We can always revisit them.

[pmachapman]

If changing the tests like that is slowing down the process too much, then let's just leave them as-is. We can always revisit them.

@Nateowami I have fixed the unit tests (I remembered how picky they were from my PR to remove blank: true.

@pmachapman reviewed 5 of 5 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @josephmyers)

[josephmyers]

Thanks, Peter. That test file is traumatizing to work in

[Nateowami]

@Nateowami reviewed 3 of 7 files at r6.
Reviewable status: 8 of 12 files reviewed, 3 unresolved discussions (waiting on @pmachapman)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.ts line 1496 at r6 (raw file):

    );
    const hasSetDraftFormatting: boolean =
      !this.featureFlagService.usfmFormat.enabled || this.draftOptionsService.isFormattingOptionsSelected();

Can you move even the check of the feature flag into the DraftOptionsService? Basically I'd like to be able to search for "usfmFormat.enabled" and find the only result is in DraftOptionsService.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.ts line 18 at r6 (raw file):

  ) {}

  isFormattingOptionsSelected(): boolean {

Can we change is => are ? (and for the getter below)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.ts line 25 at r6 (raw file):

  }

  isFormattingOptionsSupported(entry: BuildDto | undefined): boolean {

how about SupportedForBuild, for the sake of clarity?

[josephmyers]

Reviewable status: 8 of 12 files reviewed, 3 unresolved discussions (waiting on @Nateowami and @pmachapman)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.ts line 18 at r6 (raw file):

Previously, Nateowami wrote…

Can we change is => are ? (and for the getter below)

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.ts line 25 at r6 (raw file):

Previously, Nateowami wrote…

how about SupportedForBuild, for the sake of clarity?

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.ts line 1496 at r6 (raw file):

Previously, Nateowami wrote…

Can you move even the check of the feature flag into the DraftOptionsService? Basically I'd like to be able to search for "usfmFormat.enabled" and find the only result is in DraftOptionsService.

Done. I just thought it was a little weird that the service property would return true with the feature flag off. But I'm not strongly opinionated about it

[Nateowami]

@Nateowami reviewed 1 of 4 files at r7.
Reviewable status: 7 of 12 files reviewed, 2 unresolved discussions (waiting on @pmachapman)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.ts line 20 at r7 (raw file):

  areFormattingOptionsSelected(): boolean {
    return (
      !this.featureFlags.usfmFormat.enabled ||

I think you need a third method, because as written this name and implementation don't agree. Probably something like areFormattingOptionsAvailableButUnsellected. That would deal with the it was a little weird that the service property would return true with the feature flag off problem.

[josephmyers]

Reviewable status: 6 of 14 files reviewed, 1 unresolved discussion (waiting on @Nateowami and @pmachapman)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.ts line 20 at r7 (raw file):

Previously, Nateowami wrote…

I think you need a third method, because as written this name and implementation don't agree. Probably something like areFormattingOptionsAvailableButUnsellected. That would deal with the it was a little weird that the service property would return true with the feature flag off problem.

Done, I think

[Nateowami]

@Nateowami reviewed 2 of 4 files at r7, 2 of 6 files at r9.
Reviewable status: 9 of 14 files reviewed, 2 unresolved discussions (waiting on @pmachapman)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/tabs/editor-tab-menu.service.spec.ts line 331 at r9 (raw file):

    when(mockUserService.currentUserId).thenReturn('user01');
    when(mockPermissionsService.canAccessDrafts(anything(), anything())).thenReturn(true);
    when(mockFeatureFlagService.usfmFormat).thenReturn(createTestFeatureFlag(true));

Do we still need the feature flag service?

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/tabs/editor-tab-menu.service.spec.ts line 301 at r8 (raw file):

      ],
      translateConfig: {
        draftConfig: { usfmConfig: {} }

Do we still need this added?

[Nateowami]

Reviewable status: 9 of 14 files reviewed, 3 unresolved discussions (waiting on @josephmyers and @pmachapman)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.spec.ts line 89 at r10 (raw file):

      setProjectDoc(undefined);
      // Without a project doc, formatting options are implicitly unselected while flag is enabled
      expect(service.areFormattingOptionsAvailableButUnselected()).toBe(true);

Assertion does not match test name

[Nateowami]

Reviewable status: 9 of 14 files reviewed, 11 unresolved discussions (waiting on @josephmyers and @pmachapman)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.spec.ts line 9 at r10 (raw file):

import { DraftOptionsService, FORMATTING_OPTIONS_SUPPORTED_DATE } from './draft-options.service';

const activatedProjectMock = mock(ActivatedProjectService);

Throughout all our tests, our convention is to put "mocked" in front of the original name. Let's keep that for consistency and making it easier to read.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.spec.ts line 36 at r10 (raw file):

  describe('areFormattingOptionsSelected', () => {
    it('returns true when flag enabled and both options set', () => {
      setProjectDoc({ translateConfig: { draftConfig: { usfmConfig: { paragraphFormat: 'p', quoteFormat: 'q1' } } } });

Can you make this proejct doc, and other similar project docs, constants, that can be referenced? This code gets pretty repetitive, and any differences between tests can be subtle.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.spec.ts line 45 at r10 (raw file):

    });

    it('returns false when flag enabled and both options missing', () => {

at some point it feels like some tests like this are awful redundant and not worth maintaining.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.spec.ts line 56 at r10 (raw file):

    it('returns false when flag disabled even if both options set', () => {
      reset(featureFlagServiceMock);

No reset is needed.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.spec.ts line 95 at r10 (raw file):

  describe('areFormattingOptionsSupportedForBuild', () => {
    function buildWith(date: Date | undefined, flagEnabled: boolean = true): BuildDto | undefined {
      reset(featureFlagServiceMock);

Pretty sure this isn't needed.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.spec.ts line 113 at r10 (raw file):

    });

    it('returns false when date equals supported date', () => {

This is a useless test. It's testing what happens if a build finishes on the exact millisecond. We don't care. It can fall on either side. The cutoff doesn't need to be to the millisecond.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.spec.ts line 129 at r10 (raw file):

    it('returns false when entry undefined', () => {
      reset(featureFlagServiceMock);

Can be removed.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts line 87 at r9 (raw file):

    );
    when(mockedDraftOptionsService.areFormattingOptionsAvailableButUnselected()).thenReturn(true);
    when(mockedDraftOptionsService.areFormattingOptionsSupportedForBuild(anything())).thenCall((entry: BuildDto) => {

It doesn't seem like this service needs to be mocked, but maybe it's simpler to just mock it?

[josephmyers]

Reviewable status: 9 of 14 files reviewed, 7 unresolved discussions (waiting on @Nateowami and @pmachapman)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.spec.ts line 9 at r10 (raw file):

Previously, Nateowami wrote…

Throughout all our tests, our convention is to put "mocked" in front of the original name. Let's keep that for consistency and making it easier to read.

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.spec.ts line 36 at r10 (raw file):

Previously, Nateowami wrote…

Can you make this proejct doc, and other similar project docs, constants, that can be referenced? This code gets pretty repetitive, and any differences between tests can be subtle.

Done, I think. Hopefully easier to read

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.spec.ts line 45 at r10 (raw file):

Previously, Nateowami wrote…

at some point it feels like some tests like this are awful redundant and not worth maintaining.

I typically use AI to fix and write tests, so that's why this looks verbose. I can remove the ones that seem least valuable?

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.spec.ts line 56 at r10 (raw file):

Previously, Nateowami wrote…

No reset is needed.

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.spec.ts line 89 at r10 (raw file):

Previously, Nateowami wrote…

Assertion does not match test name

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.spec.ts line 95 at r10 (raw file):

Previously, Nateowami wrote…

Pretty sure this isn't needed.

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.spec.ts line 113 at r10 (raw file):

Previously, Nateowami wrote…

This is a useless test. It's testing what happens if a build finishes on the exact millisecond. We don't care. It can fall on either side. The cutoff doesn't need to be to the millisecond.

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.spec.ts line 129 at r10 (raw file):

Previously, Nateowami wrote…

Can be removed.

Done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts line 87 at r9 (raw file):

Previously, Nateowami wrote…

It doesn't seem like this service needs to be mocked, but maybe it's simpler to just mock it?

I removed it and only one test failed. So I replaced it with a simpler setup. So maybe we can just do the trivial setup?

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/tabs/editor-tab-menu.service.spec.ts line 301 at r8 (raw file):

Previously, Nateowami wrote…

Do we still need this added?

Done

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/tabs/editor-tab-menu.service.spec.ts line 331 at r9 (raw file):

Previously, Nateowami wrote…

Do we still need the feature flag service?

Done

[Nateowami]

@Nateowami reviewed 1 of 6 files at r9.
Reviewable status: 10 of 14 files reviewed, 1 unresolved discussion (waiting on @josephmyers and @pmachapman)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.spec.ts line 36 at r10 (raw file):

Previously, josephmyers wrote…

Done, I think. Hopefully easier to read

Thanks; looks much better.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.spec.ts line 45 at r10 (raw file):

Previously, josephmyers wrote…

I typically use AI to fix and write tests, so that's why this looks verbose. I can remove the ones that seem least valuable?

I do as well, so totally understandable. They just need a little more human review and cleanup before considering them done.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-options.service.spec.ts line 129 at r10 (raw file):

Previously, josephmyers wrote…

Done.

To be clear, I was saying reset can be removed. Maybe you took it as meaning the entire test?

[Nateowami]

@Nateowami reviewed 1 of 3 files at r12.
Reviewable status: 11 of 14 files reviewed, 3 unresolved discussions (waiting on @josephmyers and @pmachapman)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts line 284 at r12 (raw file):

      expect(component.buildRequestedAtDate).toBe('');
      expect(component.draftIsAvailable).toBe(false);
      expect(fixture.nativeElement.querySelector('.format-usfm')).toBeNull();

Not sure why this was removed?

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.spec.ts line 203 at r12 (raw file):

      { provide: HttpClient, useMock: mockedHttpClient },
      { provide: DraftGenerationService, useMock: mockedDraftGenerationService },
      { provide: DraftOptionsService, useMock: mockedDraftOptionsService },

Now that you've added this service, I think a lot of the other changes to this file are not needed.

[Nateowami]

Reviewable status: 11 of 14 files reviewed, 5 unresolved discussions (waiting on @josephmyers and @pmachapman)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/tabs/editor-tab-menu.service.spec.ts line 312 at r12 (raw file):

      translateConfig: {
        preTranslate: true,
        draftConfig: { usfmConfig: {} }

Not needed

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/tabs/editor-tab-menu.service.spec.ts line 322 at r12 (raw file):

    const projectDoc: SFProjectProfileDoc = explicitProjectDoc ?? this.projectDoc;
    when(activatedProjectMock.projectDoc$).thenReturn(of(projectDoc));
    when(activatedProjectMock.projectDoc).thenReturn(projectDoc as any);

Not needed

[Nateowami]

Reviewable status: 11 of 14 files reviewed, 7 unresolved discussions (waiting on @josephmyers and @pmachapman)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.spec.ts line 4663 at r12 (raw file):

      translationSuggestionsEnabled: true,
      defaultNoteTagId: 2,
      draftConfig: {

Not needed to make tests pass.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.spec.ts line 4913 at r12 (raw file):

    when(mockedFeatureFlagService.usfmFormat).thenReturn(createTestFeatureFlag(true));
    when(mockedDraftOptionsService.areFormattingOptionsAvailableButUnselected()).thenReturn(false);
    when(mockedDraftOptionsService.areFormattingOptionsSupportedForBuild(anything())).thenReturn(true);

All three lines are not needed to make tests pass.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.spec.ts line 5156 at r12 (raw file):

    }
    if (data.translateConfig?.draftConfig != null) {
      projectProfileData.translateConfig.draftConfig = data.translateConfig.draftConfig as any;

Not needed

[Nateowami]

Reviewable status: 11 of 14 files reviewed, 3 unresolved discussions (waiting on @josephmyers and @pmachapman)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/tabs/editor-tab-menu.service.spec.ts line 254 at r13 (raw file):

  it('should not show draft menu item when draft formatting (usfmConfig) is not set', done => {
    const projectDocNoFormatting = {

This setup of the project doc is unneeded, since the DraftOptionsService is mocked.

[Nateowami]

@Nateowami reviewed 1 of 3 files at r12, 1 of 2 files at r13, 1 of 1 files at r14, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @josephmyers)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.spec.ts line 284 at r12 (raw file):

Previously, Nateowami wrote…

Not sure why this was removed?

The tests pass even if I add this back...

I'm going to approve, but I'm still wondering why this was removed.