SF-3417 SF-3420 Fix project select bugs

[Nateowami]

This change is 

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.91%. Comparing base (053506b) to head (b2e244a).
Report is 2 commits behind head on master.

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3246   +/-   ##
=======================================
  Coverage   81.91%   81.91%           
=======================================
  Files         605      605           
  Lines       35009    35009           
  Branches     5697     5673   -24     
=======================================
+ Hits        28676    28677    +1     
- Misses       5487     5498   +11     
+ Partials      846      834   -12     

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

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Nateowami]

Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions

---

src/SIL.XForge.Scripture/ClientApp/src/app/project-select/project-select.component.ts line 75 at r1 (raw file):
Sharing the replay isn't just for optimization (though it has that effect). Previously the template would subscribe to this observable twice, once when the component is created, and then again when the list of filtered projects has a length greater than zero (that's the list it iterates - see the template). That means the second time it's not happening during initialization, but later. The result is that this.paratextIdControl.valueChanges.pipe(startWith('')) would fire an empty string when the actual value of the input was something else, because there was a new subscriber. It's easy to see this happening if you change the empty string to 'blah' and then go to the filtering function and have it log the search values it's getting. It gets 'blah' at the beginning as expected, but then again later when inputs have been provided.

From the shareReplay docs:

Why use shareReplay?

You generally want to use shareReplay when you have side-effects or taxing computations
that you do not wish to be executed amongst multiple subscribers.
It may also be valuable in situations where you know you will have late subscribers to
a stream that need access to previously emitted values.
This ability to replay values on subscription is what differentiates share and shareReplay.

The late subscribers and taxing computations both apply in this situation.

---

src/SIL.XForge.Scripture/ClientApp/src/app/project-select/project-select.component.ts line 72 at r1 (raw file):

    this.paratextIdControl.valueChanges.pipe(startWith('')),
    this.hiddenParatextIds$,
    this.allProjects$.pipe(startWith([]))

The main thing I needed to do to fix SF-3420 (Project select doesn't update when resource list loads) was make the filtering run when the projects or resources load. Otherwise the filtered list isn't updated until further user input.

---

src/SIL.XForge.Scripture/ClientApp/src/app/project-select/project-select.component.ts line 76 at r1 (raw file):

      });

    this.projects$

These two blocks of deleted code implement a feature that I don't think is important, and in so doing also "implement" a bug.

The idea behind it is that if you type the exact name of a project, that should be the same as selecting it from the list. I don't think it's a valuable use-case, and it results in a problem when a project name and short name are the same. If you have ABCD - ABCD in the input, and backspace until it's ABCD, this code jumps at it and says "oh, you just typed ABCD, you must want ABCD project!" and sets the value back to ABCD - ABCD.

There are 38 projects on live where name === shortName. I think there are also situations where a project that hasn't been connected yet shares a registration with another project and has an unknown name, so we use its shortName in place of its name, but I can't locate it.

[RaymondLuong3]

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Nateowami)

---

src/SIL.XForge.Scripture/ClientApp/src/app/project-select/project-select.component.ts line 72 at r1 (raw file):

Previously, Nateowami wrote…

The main thing I needed to do to fix SF-3420 (Project select doesn't update when resource list loads) was make the filtering run when the projects or resources load. Otherwise the filtered list isn't updated until further user input.

Thanks for the explanation. On a fast connection I can see how we would not see this often, and I also see why the e2e tests caught this.
It should be possible to add a unit test.

---

src/SIL.XForge.Scripture/ClientApp/src/app/project-select/project-select.component.ts line 75 at r1 (raw file):

Previously, Nateowami wrote…

Sharing the replay isn't just for optimization (though it has that effect). Previously the template would subscribe to this observable twice, once when the component is created, and then again when the list of filtered projects has a length greater than zero (that's the list it iterates - see the template). That means the second time it's not happening during initialization, but later. The result is that this.paratextIdControl.valueChanges.pipe(startWith('')) would fire an empty string when the actual value of the input was something else, because there was a new subscriber. It's easy to see this happening if you change the empty string to 'blah' and then go to the filtering function and have it log the search values it's getting. It gets 'blah' at the beginning as expected, but then again later when inputs have been provided.

From the shareReplay docs:

Why use shareReplay?

You generally want to use shareReplay when you have side-effects or taxing computations
that you do not wish to be executed amongst multiple subscribers.
It may also be valuable in situations where you know you will have late subscribers to
a stream that need access to previously emitted values.
This ability to replay values on subscription is what differentiates share and shareReplay.

The late subscribers and taxing computations both apply in this situation.

That is neat that we also get the optimization effect.

---

src/SIL.XForge.Scripture/ClientApp/src/app/project-select/project-select.component.ts line 76 at r1 (raw file):

Previously, Nateowami wrote…

These two blocks of deleted code implement a feature that I don't think is important, and in so doing also "implement" a bug.

The idea behind it is that if you type the exact name of a project, that should be the same as selecting it from the list. I don't think it's a valuable use-case, and it results in a problem when a project name and short name are the same. If you have ABCD - ABCD in the input, and backspace until it's ABCD, this code jumps at it and says "oh, you just typed ABCD, you must want ABCD project!" and sets the value back to ABCD - ABCD.

There are 38 projects on live where name === shortName. I think there are also situations where a project that hasn't been connected yet shares a registration with another project and has an unknown name, so we use its shortName in place of its name, but I can't locate it.

I agree, this is probably nice if a user has to do this operation lots of times, but in the cases where we use this I don't think many users will end up not selecting from the autocomplete panel.

[Nateowami]

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

---

src/SIL.XForge.Scripture/ClientApp/src/app/project-select/project-select.component.ts line 72 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

Thanks for the explanation. On a fast connection I can see how we would not see this often, and I also see why the e2e tests caught this.
It should be possible to add a unit test.

Done.

It has a lot more to do with the speed of the DBL than the connection speed. I can reproduce it about 100% of the time on my own machine, with no slowing of the network.

---

src/SIL.XForge.Scripture/ClientApp/src/app/project-select/project-select.component.ts line 76 at r1 (raw file):

this is probably nice if a user has to do this operation lots of times

I think it will almost always be faster to type part of the name, or the short name, and then select the project, than to type the full name exactly.

[RaymondLuong3]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Nateowami)