SF-3613 Allow old drafts to open draft tab without formatting options

[Nateowami]

Old drafts (those that don't allow selecting formatting options) should allow opening the draft tab, even when formatting options haven't been selected (since formatting options can't be selected).

Update: I just realized I really should probably be dealing with src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/tabs/editor-tab-menu.service.ts. Changes coming.

---

This change is 

[RaymondLuong3]

The editor-tab-menu.service should also be updated to allow selecting the generated draft tab if the draft is older even if usfm options have not been selected.

@RaymondLuong3 reviewed 2 of 2 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Nateowami)

---

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

  }

  areFormattingOptionsAvailableButUnselected(): boolean {

Nit: This could be a getter since it doesn't take any parameters

Code quote:

  areFormattingOptionsAvailableButUnselected(): boolean {

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.42%. Comparing base (b0eb5a1) to head (e0c71c5).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3513   +/-   ##
=======================================
  Coverage   82.42%   82.42%           
=======================================
  Files         615      615           
  Lines       36936    36951   +15     
  Branches     6039     6045    +6     
=======================================
+ Hits        30444    30457   +13     
- Misses       5595     5610   +15     
+ Partials      897      884   -13     

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

[Nateowami]

⚠️ I'm not super happy with my changes and haven't thoroughly tested them. Pushing so others can give feedback.

[RaymondLuong3]

@RaymondLuong3 reviewed 6 of 7 files at r3, 3 of 3 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Nateowami)

---

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

        const build$ = this.draftGenerationService.getLastCompletedBuild(projectId);
        return build$ == null ? of(undefined) : build$;

build$ should never be null. It will always be defined. So it would just be better to return build$.
The draft generation service listens to the project doc changes$ to determine if it needs to poll for the build progress. I think for this case, we could do the same and if a build completes and the editor tab should be no longer available (until the user selects formatting options) then we can hide the menu item. I also think it is a very specific edge case and I would be fine if we didn't dynamically update the tab menu if the user should no longer see the option for the generated draft tab.

Code quote:

        return build$ == null ? of(undefined) : build$;

[Nateowami]

Reviewable status: 8 of 10 files reviewed, 1 unresolved discussion (waiting on @RaymondLuong3)

---

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

Previously, RaymondLuong3 (Raymond Luong) wrote…

build$ should never be null. It will always be defined. So it would just be better to return build$.
The draft generation service listens to the project doc changes$ to determine if it needs to poll for the build progress. I think for this case, we could do the same and if a build completes and the editor tab should be no longer available (until the user selects formatting options) then we can hide the menu item. I also think it is a very specific edge case and I would be fine if we didn't dynamically update the tab menu if the user should no longer see the option for the generated draft tab.

🤦 Sorry. I pushed AI slop. Mea culpa, mea culpa, mea máxima culpa.

(I originally had a different implementation, and in the process of asking Copilot to fix the tests, it made changes to this file that I only glanced at and thought were probably equivalent. Clearly I need to be a lot more careful. Also worth noting that Claude Sonnet 4.5 easily fixed the tests quickly without messing things up, whereas GPT 5 Codex mangled the implementation instead, while taking a long time to do it)

[RaymondLuong3]

The code looks good, but I am getting an error when I navigate to the edit and review page and the tab menu is not showing me any options.

@RaymondLuong3 reviewed 1 of 2 files at r5, 1 of 1 files at r6, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Nateowami)

[RaymondLuong3]

Sorry, accidentally approve.

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Nateowami)

[RaymondLuong3]

It looks to be that it is working as expected now. Can you add some acceptance tests to the issue and we can get this tested.

@RaymondLuong3 reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Nateowami)

---

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

            startWith('loading' as const)
          )
    )

This could be simplified, but it might be an oversimplification.

Code quote (i):

    switchMap(projectId =>
      projectId == null
        ? of(null)
        : this.draftGenerationService.getLastCompletedBuild(projectId).pipe(
            map(build => build ?? null),
            startWith('loading' as const)
          )
    )

Code snippet (ii):

    switchMap(projectId =>
      projectId == null ? of(undefined) : this.draftGenerationService.getLastCompletedBuild(projectId)
    )

[Nateowami]

Reviewable status: 7 of 10 files reviewed, 1 unresolved discussion (waiting on @RaymondLuong3)

---

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

Previously, RaymondLuong3 (Raymond Luong) wrote…

This could be simplified, but it might be an oversimplification.

I'm not sure why I thought we needed to make it this complicated; the behavior is the same when the build hasn't loaded yet, and when it's not available. I've updated it.

[Nateowami]

@RaymondLuong3 Can you personally test this, since setting up the situation requires having an old build, or editing the FORMATTING_OPTIONS_SUPPORTED_DATE variable?

[RaymondLuong3]

I will do some more testing tomorrow, but I think we should move this to testers. I'll write an acceptance test for this.

@RaymondLuong3 reviewed 3 of 3 files at r8, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Nateowami)

---

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

Previously, Nateowami wrote…

I'm not sure why I thought we needed to make it this complicated; the behavior is the same when the build hasn't loaded yet, and when it's not available. I've updated it.

That looks good.

[Nateowami]

Reviewable status: 8 of 10 files reviewed, all discussions resolved (waiting on @RaymondLuong3)

---

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

Previously, RaymondLuong3 (Raymond Luong) wrote…

That looks good.

Thinking about this more, I had some concerns about always loading the draft build, even when a user doesn't have permission to view the draft (likely to cause an error), when offline, or a project doesn't even use drafting. I've updated the logic to what I think should handle this correctly. It's supposed to now only load it when a user has permission to view the draft. Additionally, it listens to changes to the project doc, so if a draft were to complete and make the draft available, it would react to that, which is a nice bonus.

[RaymondLuong3]

I've done a bit more testing. It seems to be working with just one minor issue about realtime updates.

@RaymondLuong3 reviewed 2 of 2 files at r9, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Nateowami)

---

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

Previously, Nateowami wrote…

Thinking about this more, I had some concerns about always loading the draft build, even when a user doesn't have permission to view the draft (likely to cause an error), when offline, or a project doesn't even use drafting. I've updated the logic to what I think should handle this correctly. It's supposed to now only load it when a user has permission to view the draft. Additionally, it listens to changes to the project doc, so if a draft were to complete and make the draft available, it would react to that, which is a nice bonus.

Accounting for the permission service and the latest build is a good idea. I'll continue testing around these edge cases too.

---

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

@Injectable()
export class EditorTabMenuService implements TabMenuService<EditorTabGroupType> {
  private readonly hasDraftAndPermission$: Observable<boolean> = this.activatedProject.projectDoc$.pipe(

If you want it to work when the project doc is updated, you will need to subscribe to projectDoc.changes$.

Code quote:

  private readonly hasDraftAndPermission$: Observable<boolean> = this.activatedProject.projectDoc$.pipe(

---

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

        return combineLatest([of(projectDoc), of(isOnline), of(showDraftTab), this.tabState.tabs$]);
      }),
      switchMap(([projectDoc, isOnline, showDraftTab, existingTabs]) => {

This looks overly complicated. We can remove one of the switchMaps and I think it would be logically the same.

Code quote (i):

    return combineLatest([
      this.activatedProject.projectDoc$.pipe(filterNullish()),
      this.onlineStatus.onlineStatus$,
      this.showDraftTab$
    ]).pipe(
      quietTakeUntilDestroyed(this.destroyRef),
      switchMap(([projectDoc, isOnline, showDraftTab]) => {
        return combineLatest([of(projectDoc), of(isOnline), of(showDraftTab), this.tabState.tabs$]);
      }),
      switchMap(([projectDoc, isOnline, showDraftTab, existingTabs]) => {

Code snippet (ii):

    return combineLatest([
      this.activatedProject.projectDoc$.pipe(filterNullish()),
      this.onlineStatus.onlineStatus$,
      this.showDraftTab$,
      this.tabState.tabs$
    ]).pipe(
      quietTakeUntilDestroyed(this.destroyRef),
      switchMap(([projectDoc, isOnline, showDraftTab, existingTabs]) => {
        const items: Observable<TabMenuItem>[] = [];

[Nateowami]

Reviewable status: 8 of 10 files reviewed, 2 unresolved discussions (waiting on @RaymondLuong3)

---

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

Previously, RaymondLuong3 (Raymond Luong) wrote…

If you want it to work when the project doc is updated, you will need to subscribe to projectDoc.changes$.

Good point. That's what I had meant to use. Updated (also updated lower in the file).

---

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

Previously, RaymondLuong3 (Raymond Luong) wrote…

This looks overly complicated. We can remove one of the switchMaps and I think it would be logically the same.

Unfortunately it seems it's not actually equivalent, though I'm not sure why either.

[RaymondLuong3]

@RaymondLuong3 reviewed 2 of 2 files at r10, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Nateowami)

---

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

Previously, Nateowami wrote…

Unfortunately it seems it's not actually equivalent, though I'm not sure why either.

That is interesting. Must be a reason why it was initially written this way. Thanks for checking.

[Nateowami]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Nateowami)

---

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

Previously, RaymondLuong3 (Raymond Luong) wrote…

Nit: This could be a getter since it doesn't take any parameters

Personally I think it's better as-is...

---

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

Previously, RaymondLuong3 (Raymond Luong) wrote…

That is interesting. Must be a reason why it was initially written this way. Thanks for checking.

@RaymondLuong3 I've updated the tests to allow this. Moving this.tabState.tabs$ earlier means it was evaluated when the service is constructed, and the mocks in the test weren't setting mockTabState.tabs$ until it was too late.

[RaymondLuong3]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Nateowami)

---

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

Previously, Nateowami wrote…

@RaymondLuong3 I've updated the tests to allow this. Moving this.tabState.tabs$ earlier means it was evaluated when the service is constructed, and the mocks in the test weren't setting mockTabState.tabs$ until it was too late.

Ok, the changes I see here appear to be the case without the suggestion that I made. Did you forget to push the changes?

---

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

  private initMenuItems(): Observable<TabMenuItem[]> {
    return combineLatest([
      this.activatedProject.changes$.pipe(filterNullish()),

In my testing, this did not trigger the menu items to update after the user saved the formatting options. I'm not sure why that is the case. But I don't think that needs to be a blocker.

Code quote:

      this.activatedProject.changes$.pipe(filterNullish()),

[Nateowami]

I am going to merge this so it makes this week's QA release.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Nateowami)

---

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

Previously, RaymondLuong3 (Raymond Luong) wrote…

Ok, the changes I see here appear to be the case without the suggestion that I made. Did you forget to push the changes?

Yes, I think I forgot to push the changes. They can be in a followup.