SF-2788 Reduce calls to the Paratext Registry

[pmachapman]

This PR reduces calls to the Paratext Registry by combining:

• ParatextService.GetProjectRolesAsync()
• ParatextService.GetParatextUsernameMappingAsync()

Into ParatextService.GetUsersAsync(), and by passings the list of users from the Registry to ParatextNotesMapper, rather than having ParatextNotesMapper request the list of users from the Registry again.

I have also added a second commit that checks whether a project is registered, and if so, uses just that project's meta data and license to retrieve the repositories from the PT archives server.

---

This change is 

[codecov]

Codecov Report

Attention: Patch coverage is 69.07895% with 47 lines in your changes missing coverage. Please review.

Project coverage is 78.33%. Comparing base (abd834d) to head (b62012c).

Files	Patch %	Lines
...ture/Services/JwtInternetSharedRepositorySource.cs	0.00%	26 Missing ⚠️
...c/SIL.XForge.Scripture/Services/ParatextService.cs	74.35%	12 Missing and 8 partials ⚠️
.../SIL.XForge.Scripture/Services/SFProjectService.cs	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2529      +/-   ##
==========================================
+ Coverage   78.26%   78.33%   +0.07%     
==========================================
  Files         517      518       +1     
  Lines       29718    29637      -81     
  Branches     4824     4840      +16     
==========================================
- Hits        23258    23217      -41     
+ Misses       5721     5674      -47     
- Partials      739      746       +7     

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

[pmachapman]

Dismissed @github-advanced-security[bot] from a discussion.
Reviewable status: 0 of 12 files reviewed, all discussions resolved (waiting on @github-advanced-security[bot])

[nigel-wells]

Looks like I didn't push these comments through the other day when I hit issues trying to test. I'm pushing them now - still to finish further testing.

Reviewed 9 of 12 files at r1, 1 of 3 files at r2, 4 of 4 files at r3, 7 of 7 files at r4, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @pmachapman)

---

src/SIL.XForge.Scripture/Services/JwtInternetSharedRepositorySource.cs line 243 at r3 (raw file):

    }

    private JObject? GetJsonObject(string cgiCall)

Can the fetching, handling, and logging of this be merged with that in JArray above? Seems like they both doing the same thing and we just need to return something different so long as nothing was thrown.

---

src/SIL.XForge.Scripture/Services/ParatextService.cs line 626 at r3 (raw file):

        if (IsResource(project.ParatextId))
        {
            // Resources don't have project members or roles

nit can we add a return here rather than having to read the rest of the code to see if something else of importance is going to happen.

---

src/SIL.XForge.Scripture/Services/ParatextService.cs line 647 at r3 (raw file):

            );

            WarnIfNonuniqueValues(

Have you ruled out that this is no longer a possibility? I'm not really sure what scenario it was trying to log but it was done for a reason. Maybe it relates to our duplicate account issue when we do an automatic merge with Auth0 accounts which leaves redundant SF users but 2x PT IDs?

---

test/SIL.XForge.Scripture.Tests/Services/ParatextNotesMapperTests.cs line 337 at r3 (raw file):

        var env = new TestEnvironment();
        env.SetParatextProjectRoles(false);
        env.InitMapper(false, false);

Why no longer testing if this is true?

[pmachapman]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @nigel-wells)

---

src/SIL.XForge.Scripture/Services/JwtInternetSharedRepositorySource.cs line 243 at r3 (raw file):

Previously, nigel-wells (Nigel Wells) wrote…

Can the fetching, handling, and logging of this be merged with that in JArray above? Seems like they both doing the same thing and we just need to return something different so long as nothing was thrown.

Done. Thank you for spotting that - I have made it a generic.

---

src/SIL.XForge.Scripture/Services/ParatextService.cs line 626 at r3 (raw file):

Previously, nigel-wells (Nigel Wells) wrote…

nit can we add a return here rather than having to read the rest of the code to see if something else of importance is going to happen.

Done. I have added a return at the end of each block of the if...else to prevent any potential future problems with logic added after the if block.

---

src/SIL.XForge.Scripture/Services/ParatextService.cs line 647 at r3 (raw file):
I checked the production database, and there were no occurrences of users with duplicate paratextIds. I also grepped the journal (since April), and the error is not present in the journal.

QA has two duplicate users, dating back to 2021, which should be removed (one of them is an account created by you). I will need your help to clean these up.

The code was added in 2022 - #1314. See the comment from Ira:

I presume this is a temporary addition to find a specific issue and will be reverted once resolved.

---

test/SIL.XForge.Scripture.Tests/Services/ParatextNotesMapperTests.cs line 337 at r3 (raw file):

Previously, nigel-wells (Nigel Wells) wrote…

Why no longer testing if this is true?

Because in the original test, it incorrectly relied on their being inconsistent output from the mock Paratext API to not map "user03" to "PT User 3". This was a test setup issue, and so when I corrected the mock registry to output consistent information, the test failed.

As the test is testing what happens when a Paratext user is not on the project, I updated the twoPtUsersOnProject argument for InitMapper() to be false, and so only "user01" is the only Paratext user.

[nigel-wells]

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

---

src/SIL.XForge.Scripture/Services/ParatextService.cs line 647 at r3 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

I checked the production database, and there were no occurrences of users with duplicate paratextIds. I also grepped the journal (since April), and the error is not present in the journal.

QA has two duplicate users, dating back to 2021, which should be removed (one of them is an account created by you). I will need your help to clean these up.

The code was added in 2022 - #1314. See the comment from Ira:

I presume this is a temporary addition to find a specific issue and will be reverted once resolved.

Happy with this as discussed.