SF-3759 Add Slope and Intercept input to Serval Admin

[pmachapman]

I have marked as do not merge, as the final logic for Quality Estimation needs to be confirmed. This PR is ready for review, however.

---

This change is 

---

[codecov]

Codecov Report

❌ Patch coverage is 95.91837% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.39%. Comparing base (d753095) to head (ae7999b).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
.../SIL.XForge.Scripture/Services/SFProjectService.cs	92.00%	0 Missing and 2 partials ⚠️
.../serval-administration/serval-project.component.ts	97.22%	0 Missing and 1 partial ⚠️
...e.Scripture/Controllers/SFProjectsRpcController.cs	96.87%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3766      +/-   ##
==========================================
+ Coverage   81.30%   81.39%   +0.08%     
==========================================
  Files         622      623       +1     
  Lines       39390    39470      +80     
  Branches     6390     6402      +12     
==========================================
+ Hits        32028    32127      +99     
+ Misses       6377     6355      -22     
- Partials      985      988       +3     

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

[marksvc]

@marksvc reviewed 15 files and all commit messages, and made 3 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on pmachapman).

---

src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/serval-project.component.ts line 397 at r4 (raw file):
(Just saying.) It occurs to me that if the user enters

{
  "version": "0.1",
  "slope": 1.1,
  "intercept": 1.1,
  "something": "else"
}

, that the "something": "else" will be passed from the frontend to the server. I expect that he server, being C#, will not include the something field and it won't be a problem.

Devin agrees that it will not be a problem. And mentions

The JSON-RPC framework deserializes the payload into a QualityEstimationConfig C# object. By default, extra JSON properties are silently ignored during deserialization — something is dropped. The C# object only has Version, Slope, and Intercept.

---

src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/serval-project.component.ts line 379 at r4 (raw file):

      projectId == null ||
      (this.form.value.qualityEstimationConfig ?? '') ===
        (project.translateConfig.draftConfig.qualityEstimationConfig ?? '')

Hmm. Okay. This is a bit of an odd construct with the types. But I see how it works. Helped by the comment above.

You might consider the following as well, if you don't prefer the current lines.

      (project.translateConfig.draftConfig.qualityEstimationConfig == null &&
        !isPopulatedString(this.form.value.qualityEstimationConfig))

Hmm, I see that the updateServalConfig below is written in a similar way.

---

src/SIL.XForge.Scripture/Models/QualityEstimationConfig.cs line 8 at r4 (raw file):

public class QualityEstimationConfig
{
    public required string Version { get; set; }

I've done some searching but am still a bit confused about the relationship between required and this type being non-nullable. What is the motivation to specify required here?

[pmachapman]

@pmachapman made 2 comments and resolved 1 discussion.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on marksvc).

---

src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/serval-project.component.ts line 379 at r4 (raw file):

Previously, marksvc wrote…

Hmm. Okay. This is a bit of an odd construct with the types. But I see how it works. Helped by the comment above.

You might consider the following as well, if you don't prefer the current lines.

      (project.translateConfig.draftConfig.qualityEstimationConfig == null &&
        !isPopulatedString(this.form.value.qualityEstimationConfig))

Hmm, I see that the updateServalConfig below is written in a similar way.

I think I should stick with the current logic, as it checks for empty equivalence and null equivalence, and does not update if there is no change in that regards.

---

src/SIL.XForge.Scripture/Models/QualityEstimationConfig.cs line 8 at r4 (raw file):

Previously, marksvc wrote…

I've done some searching but am still a bit confused about the relationship between required and this type being non-nullable. What is the motivation to specify required here?

required just means that it must be populated with a string, i.e. not null. That means that at the very least, this will be string.Empty.

I could have written it as a pre-instantiated property (as well as nullable, of course), i.e.

public string Version { get; set; } = string.Empty;

[marksvc]

@marksvc reviewed 1 file and all commit messages, and resolved 2 discussions.
Reviewable status: 14 of 15 files reviewed, all discussions resolved.