Draft jobs duration mean: only include successful jobs

[marksvc]

The Max still includes all jobs. This is to assist with metrics
reporting.

---

This screenshot shows the Mean and Max display area on the Draft Jobs table.

---

This change is 

[codecov]

Codecov Report

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

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3611   +/-   ##
=======================================
  Coverage   82.82%   82.82%           
=======================================
  Files         610      610           
  Lines       37398    37400    +2     
  Branches     6151     6151           
=======================================
+ Hits        30974    30976    +2     
+ Misses       5491     5478   -13     
- Partials      933      946   +13     

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

[RaymondLuong3]

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

---

src/SIL.XForge.Scripture/ClientApp/src/type-utils.ts line 140 at r1 (raw file):

export function isPopulatedString(value: unknown): value is Exclude<string, ''> {
  return typeof value === 'string' && value !== '';
}

I'm not sure what the purpose of moving this from utils is. Why is this a more appropriate place for this code?

Code quote:

export function isPopulatedString(value: unknown): value is Exclude<string, ''> {
  return typeof value === 'string' && value !== '';
}

---

src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs-export.service.ts line 138 at r1 (raw file):

    // Append max duration row
    const maxRow: SpreadsheetRow = {
      servalBuildId: 'Max any duration minutes',

nit: I like max all better

Code quote:

'Max any duration minutes',

[marksvc]

Thank you. Here is an updated screenshot.

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

---

src/SIL.XForge.Scripture/ClientApp/src/type-utils.ts line 140 at r1 (raw file):
Right, good question. When I put isPopulatedString into xforge-common/utils.ts, I think it was not the right choice. The xforge-common/utils.ts file is mostly populated by browser-related or other higher level utilities. Whereas type-utils.ts is mostly populated by types and type guards that work with TypeScript language-level details.

isPopulatedString acts not only to check and guide program flow based on the content of a string, but it also provides a type guard to inform the type system as to what the shape of the value in question can be.[*]

So I think it is better placed with similar code in type-utils.ts.

[*] Then you can do things like:

  get draftGenerationRequestIdDisplay(): string {
    if (!isPopulatedString(this.data.draftGenerationRequestId)) return 'none';
    return this.data.draftGenerationRequestId;
  }

If the isPopulatedString just had a boolean return type, like

export function isPopulatedString(value: unknown): boolean {
  return typeof value === 'string' && value !== '';
}

, then the get draftGenerationRequestIdDisplay getter would have an error on the second return, saying

Type 'string | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.

But with the type guard (value is Exclude<string, ''>), TypeScript is instructed that if isPopulatedString returns true, it should treat the value as not possibly being null, and so it does not have the error. [**]

[**] Exclude<string, ''>here is like "type string, except not the empty string".
(Also, in TypeScript, null and undefined have different types than a string. So if something is of type string, then it is not null or undefined.)

---

src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs-export.service.ts line 138 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

nit: I like max all better

Done. I wrestled with how I might word these to be consistent, and whether they should contain "of", etc. I left Mean as it is and changed max to "Max all durations" and "Max all durations minutes". What do you think of this?

[RaymondLuong3]

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

---

src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs-export.service.ts line 138 at r1 (raw file):

Previously, marksvc wrote…

Done. I wrestled with how I might word these to be consistent, and whether they should contain "of", etc. I left Mean as it is and changed max to "Max all durations" and "Max all durations minutes". What do you think of this?

That works!