SF-3545 Add per-user lynx toggle to translator settings

[siltomato]

Admin lynx toggles are on the Settings page, but translators may want to disable certain lynx features. This PR adds lynx toggles to the Translator Settings dialog, which previously only held translation suggestions settings.

• Added lynx toggles to the Translator Settings dialog
  • Lynx settings here should only be visible if lynx is enabled in project settings
  • This translation settings 'cog' should now be visible if lynx is enabled in project settings, but translation suggestions are not
• Reworked the Translator Settings dialog to use mat-cards

---

This change is 

[codecov]

Codecov Report

❌ Patch coverage is 84.61538% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.13%. Comparing base (91d70de) to head (8bdebb1).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...ate/editor/translator-settings-dialog.component.ts	89.28%	4 Missing and 2 partials ⚠️
...ntApp/src/app/translate/editor/editor.component.ts	71.42%	1 Missing and 3 partials ⚠️
...SIL.XForge.Scripture/Models/LynxInsightUserData.cs	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3426      +/-   ##
==========================================
+ Coverage   82.08%   82.13%   +0.05%     
==========================================
  Files         611      611              
  Lines       36314    36350      +36     
  Branches     5990     5973      -17     
==========================================
+ Hits        29807    29857      +50     
+ Misses       5642     5629      -13     
+ Partials      865      864       -1     

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

[pmachapman]

@pmachapman reviewed 20 of 20 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @siltomato)

---

src/RealtimeServer/scriptureforge/services/sf-project-user-config-migrations.ts line 106 at r1 (raw file):

      const op: ObjectInsertOp = {
        p: ['lynxUserConfig'],
        oi: { assessmentsEnabled: true, autoCorrectionsEnabled: true }

This migration sets this to true, but the C# object will make these properties' values null for any new records.

I think you should not populate these two properties, as you already coalesce null to true elsewhere, and I think the "undefined" state for these properties is very useful to show that a user has not configured them. i.e.

oi: {}

Code quote:

oi: { assessmentsEnabled: true, autoCorrectionsEnabled: true }

---

src/RealtimeServer/scriptureforge/services/sf-project-user-config-migrations.ts line 111 at r1 (raw file):

    }
  }
}

Can you please add a test for this?

Also in the previous PR I forgot to ask for a test for SFProjectMigration25 - if you are able to add tests for that migration too, that would help tie up a loose end.

Code quote:

class SFProjectUserConfigMigration9 extends DocMigration {
  static readonly VERSION = 9;

  async migrateDoc(doc: Doc): Promise<void> {
    if (doc.data.lynxUserConfig === undefined) {
      const op: ObjectInsertOp = {
        p: ['lynxUserConfig'],
        oi: { assessmentsEnabled: true, autoCorrectionsEnabled: true }
      };
      await submitMigrationOp(SFProjectUserConfigMigration9.VERSION, doc, [op]);
    }
  }
}

---

src/RealtimeServer/scriptureforge/models/lynx-config.ts line 11 at r1 (raw file):

  autoCorrectionsEnabled?: boolean;
  assessmentsEnabled?: boolean;
}

This interface should be either in a separate ts file, or perhaps in lynx-insight-user-data.ts if you make this interface a property of LynxInsightUserData.

Code quote:

export interface LynxUserConfig {
  autoCorrectionsEnabled?: boolean;
  assessmentsEnabled?: boolean;
}

---

src/RealtimeServer/scriptureforge/models/sf-project-user-config.ts line 34 at r1 (raw file):

  editorTabsOpen: EditorTabPersistData[];
  lynxInsightState: LynxInsightUserData;
  lynxUserConfig: LynxUserConfig;

Should lynxUserConfig be a property of lynxInsightState?

I notice that object is a bit bare, and perhaps you had planned for stuff like this to be in it?

Code quote:

  lynxInsightState: LynxInsightUserData;
  lynxUserConfig: LynxUserConfig;

[Nateowami]

In my testing, it is not possible to open the dialog to configure the settings unless a source project is configured. This should be fixed before this change is complete.

[siltomato]

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

---

src/RealtimeServer/scriptureforge/models/lynx-config.ts line 11 at r1 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

This interface should be either in a separate ts file, or perhaps in lynx-insight-user-data.ts if you make this interface a property of LynxInsightUserData.

Moved properties directly under LynxInsightUserData, and removed LynxUserConfig.

---

src/RealtimeServer/scriptureforge/models/sf-project-user-config.ts line 34 at r1 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

Should lynxUserConfig be a property of lynxInsightState?

I notice that object is a bit bare, and perhaps you had planned for stuff like this to be in it?

Yes, this is a good point. I'll consolidate the two.

---

src/RealtimeServer/scriptureforge/services/sf-project-user-config-migrations.ts line 106 at r1 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

This migration sets this to true, but the C# object will make these properties' values null for any new records.

I think you should not populate these two properties, as you already coalesce null to true elsewhere, and I think the "undefined" state for these properties is very useful to show that a user has not configured them. i.e.

oi: {}

Removed migration #9, as properties are now under lynxInsightState and migration #8 creates lynxInsightState. Also added a missing migration test for #8.

---

src/RealtimeServer/scriptureforge/services/sf-project-user-config-migrations.ts line 111 at r1 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

Can you please add a test for this?

Also in the previous PR I forgot to ask for a test for SFProjectMigration25 - if you are able to add tests for that migration too, that would help tie up a loose end.

SFProjectUserConfigMigration9 is now removed (due to refactor), but I added a migration test for SFProjectMigration25.

[pmachapman]

@pmachapman reviewed 16 of 16 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @siltomato)

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/translator-settings-dialog.component.ts line 157 at r2 (raw file):

  private updateLynxInsightState(updates: { assessmentsEnabled?: boolean; autoCorrectionsEnabled?: boolean }): void {
    this.projectUserConfigDoc.submitJson0Op(op => {

Should this promise be awaited (and all the method calls up the chain, and setTranslationSettingsEnabled)? If not, can the two calls to submitJson0Op in this component be decorated with void?

Code quote:

this.projectUserConfigDoc.submitJson0Op

---

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

  get translatorSettingsEnabled(): boolean {
    return (this.translationSuggestionsProjectEnabled || this.lynxProjectEnabled) && this.userHasGeneralEditRight;

We still need the two source properties to correctly determine if there are translation suggestions, i.e. the logic should be something along the lines of:

return ((this.hasSource && this.hasSourceViewRight && this.translationSuggestionsProjectEnabled) || this.lynxProjectEnabled) && this.userHasGeneralEditRight;

Code quote:

return (this.translationSuggestionsProjectEnabled || this.lynxProjectEnabled) && this.userHasGeneralEditRight;

[siltomato]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @pmachapman)

---

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

Previously, pmachapman (Peter Chapman) wrote…

We still need the two source properties to correctly determine if there are translation suggestions, i.e. the logic should be something along the lines of:

return ((this.hasSource && this.hasSourceViewRight && this.translationSuggestionsProjectEnabled) || this.lynxProjectEnabled) && this.userHasGeneralEditRight;

Yes, thanks, much better. I opened the translator settings, and the suggestions section was hidden, but I should have enforced it here.

---

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/translator-settings-dialog.component.ts line 157 at r2 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

Should this promise be awaited (and all the method calls up the chain, and setTranslationSettingsEnabled)? If not, can the two calls to submitJson0Op in this component be decorated with void?

The explicit 'void' is a great idea. Added.

[Nateowami]

If I turn off assessments, but leave Lynx on, the assessments don't disappear.

Reviewable status: 22 of 25 files reviewed, 3 unresolved discussions (waiting on @pmachapman)

---

src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json line 751 at r3 (raw file):

    "lynx_enable_assessments": "Show assessments",
    "lynx_enable_autocorrect": "Allow auto-corrections",
    "lynx_settings_title": "Lynx",

Needs to be updated to "Insights"

[siltomato]

• Blots and overlays should now clear when insights are disabled.
• Changed 'lynx' to 'insights' in translation file.

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

---

src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json line 751 at r3 (raw file):

Previously, Nateowami wrote…

Needs to be updated to "Insights"

Done.

Requested changes have been addressed.

[pmachapman]

@pmachapman reviewed 2 of 3 files at r3, 5 of 5 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @siltomato)