search

wordpress-mobile / WordPress-Login-Flow-Android

Pluggable WordPress login flow for Android
GNU General Public License v2.0
14 stars 3 forks source link

Filtering out CP connected sites for WPMobile only. #76

Closed develric closed 2 years ago

develric commented 2 years ago

IMPORTANT: this is ready to be reviewed; but since it has implications for Woo and JetPack as well, keeping it as a draft so it is not accidentally merged before the validation from WP/JP Android and WooAndroid point of view.

Part of WPMobile 15540 this PR aims to filter out the CP connected sites for WPMobile only.

There is a FluxC and a WPAndroid companion PRs.

Appreciate all the testing you could provide for the login/signup flow paths.

For WPAndroid specific issue, you can follow the test instructions in the companion relevant PR above. @ashiagr note that for JetPack app I'm not filtering out the CP connected sites for now, since sites that have the Jetpack Backup plugin (even without the full JP) fall under this category but I think should be managed by the Jetpack app, whereas those kind of sites as of now should be added as self-hosted sites in WPAndroid. Not sure though about what kind of issues those sites could create for the Jetpack app.

Note that the FluxC PR is not a breaking one, since it allows legacy behaviour with overloads; but AFAIU when Woo is going to update to this new version of the login library, we need Woo to also update the Fluxc to be >= trunk-9f5f5ea43dd60152677092b499980898f9521f6c So that the linked fluxc contains the overloaded version of FetchSitesPayload. I think that makes sense to have a woo small PR to already bring this ahead, so to not forget about it and have problems later. Wdyt @hichamboushaba ?

NOTE: The fluxc version linked from the login lib is pretty old (1.23.0); I linked to the current trunk and smoke tested the flow paths at my best without issues, but as said I would appreciate other pair of eyes on the different login/signup flow paths for all the 3 apps 🙇 .

hichamboushaba commented 2 years ago

So that the linked fluxc contains the overloaded version of FetchSitesPayload. I think that makes sense to have a woo small PR to already bring this ahead, so to not forget about it and have problems later. Wdyt @hichamboushaba ?

I agree @develric, if you want me to create the Woo PR, please just ping me after this PR gets reviewed, and I can do it.

develric commented 2 years ago

Hi @hichamboushaba 👋 , we tested/reviewed this PR now; was wondering if you could be still available to create the relevant Woo PR? I'll wait to move this to open/merged after your follow up, thanks 🙇