You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

You can test the changes on this Pull Request by downloading the APK here.

tl;dr I think we can keep things as they are, but I had to explore things deeply in order to understand whether the changes here (or even the code as it was in some cases) are in the right direction in terms of code design. I have thoughts about it but aligning to these thoughts most probably means a bigger refactor that exceeds the purpose of this PR. Sharing my thoughts below, feel free to save for later.

After a couple of iterations approaching changes to these classes involved in this PR I felt I had a better understanding of how things work here, so I was wondering whether passing a SiteModel to a ViewModel was technically advised, since when the site changes we need to refresh it somehow and make sure the UI changes are triggered, propagating / representing the changes to the ViewModel on the View.
The trigger for these thoughts was that I couldn't find examples in the codebase that were using a SiteModel being passed to a ViewModel, but found plenty of examples of SiteStore being injected in ViewModels, and then obtaining the SiteModel from the Store via its localId. This is probably a better way in which the VM knows about the Repository, and the Repository is responsible for entity models (i.e. in this case a SiteModel).

Some examples:

• DomainRegistrationDetailsViewModel: injects SiteStore, gets the actual model from re-obtaining it from the store, via a passed SiteModel's id in start()
• StorePostViewModel -> same
• HomePageSettingsViewModel -> same
• StatsSiteSelectionViewModel -> same
• ReaderViewModel -> injects AccountStore
  etc.

Then I realized we're never keeping the selected site on WPMainActivityViewModel, but on WPMainActivity (which is weird, ideally the selectedSite would be kept in a Repository and the ViewModel would be able to interact with it as per MVVM architecture). So this is beyond the scope of this PR but it feels what we should really be doing here is probably modifying things to keep the state in the VM, instead of waiting for the user to click on the FAB to update the state and then reflect it on the screen.

StatsViewAllViewModel has a good example of this in which it gets a StatsSiteProvider injected in its constructor parameters, which in turn encapsulates the interaction in a Repository (in particular the very simple SelectedSiteStorage), and the Provider gets the updates from registering to FluxC OnSiteChanged events itself.
See #9353 and the related issue description (#9352) for a bit more reasoning on why passing the SiteModel may be not ideal - it was probably identified as an edge case but it makes things more robust.
(another ViewModel using that is InsightsManagementViewModel for example).

If we went down that route, I think not only we would get rid of passing SiteModel, but also would benefit of not having to pass booleans by encapsulating the selectedSite capabilities check logic in the ViewModel, and just rely on the VM and the Activity doing their business as usual given by triggering refreshes though LiveData observers.

Open to discussing thoughts and learning from your points of view, but to make it clear I wouldn't block this PR with this discussion 😄

So this is beyond the scope of this PR but it feels what we should really be doing here is probably modifying things to keep the state in the VM, instead of waiting for the user to click on the FAB to update the state and then reflect it on the screen.

I agree completely, I wasn't a big fan of my solution but it felt like an okay interim solution vs adding another boolean to pass to every method. In most other places I'd have gone for injecting the SiteStore as well, but as you outlined the site would have to be updated whenever there's a change, and the selected site really should live in the ViewModel, requiring a bigger refactor and a lot more care since this is a central part of the app and selectedSite logic has given us plenty of trouble in the past. I think that's definitely worth doing but out of scope for this PR.

3. Jetpack-connected site, version < 8.8 -> Story option not shown 🙅‍♂️ couldn't test this one but did step by step debugging and observed the checks for minimum version run correctly

👍 Just for future use, I forgot to mention that an easy way to test earlier Jetpack versions is to install the Jetpack beta plugin, that allows you to switch to earlier releases.

I agree completely, I wasn't a big fan of my solution but it felt like an okay interim solution vs adding another boolean to pass to every method. In most other places I'd have gone for injecting the SiteStore as well, but as you outlined the site would have to be updated whenever there's a change, and the selected site really should live in the ViewModel, requiring a bigger refactor and a lot more care since this is a central part of the app and selectedSite logic has given us plenty of trouble in the past. I think that's definitely worth doing but out of scope for this PR.

That makes sense - let's merge this then, since it works as expected 😎