wordpress-mobile / WordPressAuthenticator-iOS

GNU General Public License v2.0
17 stars 11 forks source link

Confirm that site is indeed not a WordPress site after XML-RPC validation fails #796

Closed staskus closed 10 months ago

staskus commented 12 months ago

More context: https://github.com/wordpress-mobile/WordPress-iOS/issues/18497#issuecomment-1779361108 Related PR https://github.com/wordpress-mobile/WordPress-iOS/pull/21904

In rare cases, XML-RPC validation can fail and indicate that the site is not a WordPress site when it actually is.

To avoid this corner case, I added an additional isWPSite check after guessXMLRPCURL and before showing errors to the users:

Implementation details

I didn't want to change this fragile code before writing tests to it and to write tests I needed to split business logic from UI. I recommend reviewing commit by commit:

  1. Extract error handling code - https://github.com/wordpress-mobile/WordPressAuthenticator-iOS/pull/796/commits/9c14098b737397095420c4bd68cc39ebe1ce8778
  2. Move it to ViewModel - https://github.com/wordpress-mobile/WordPressAuthenticator-iOS/pull/796/commits/b62db739cd3f6d6b20e50947ccc6821c8309e32b
  3. Add unit tests - https://github.com/wordpress-mobile/WordPressAuthenticator-iOS/pull/796/commits/76925ba83e7a6712b7412fc7d849bbb6de457ab8
  4. Add isWPSite check - https://github.com/wordpress-mobile/WordPressAuthenticator-iOS/pull/796/commits/a27ac06167677a7303a4d84d633c0daa6b82fb45
staskus commented 12 months ago

@guarani the code can be reviewed but ideally, I would do some more manual testing myself, so I'm not targeting 23.6 for sure. If you'll have time to review the code or critique the solution overall - great, I'll finish after I come back. Thanks! 🙇