WordPress-Android
WordPress-Android copied to clipboard
Match Api domain sanitization algorithm to avoid listing available domains as unavailable
Fixes #15185
This PR brings the domain sanitization algorithm from the domains/suggestions API to the mobile app in order to fix an issue where *.wordpress.com subdomains would be displayed as unavailable first domain suggestion, although the API returns them as available and valid domains.
The UX could be further improved given the following known issues, but I left that out of scope due to timing reasons since for now this change already feels like an improvement to me since we're matching the API algorithm; further improvements most probably need updates to the API side as well.
Known issues:
- Searching for anything in the range of
testingtfm3.wtotestingtfm3.wordpreswill not listtestingtfm3.wordpres.comin the search results. Ideally this would work intuitively, listing thetestingtfm3.wordpres.commatch closer to the top as we're adding more characters for the domain name. - Searching for
testingtfm3.wordpress.comwill not show it as the first result, while searching fortestingtfm3.wordpressdoes. - Searching for
testingtfm3.wordpres.coshows the first result astestingtfm3.wordpres.co.wordpres.com. This feels a tad off to me but out of scope.
To test:
- Open Jetpack App from PR (QR-code or local build)
- Start the Site Creation flow
- Skip until the the
Choose a domainscreen is shown - Search for a domain name (E.g.
testingtfm3)- Expect
testingtfm3ord.wordpress.comto be listed as available (scroll down if needed)
- Expect
- Continue typing more letters of the
wordpress(E.g.testingtfm3.w..testingtfm3.wordpres)- Expect To not see
testingtfm3.wordpress.comas first result flagged as unavailable ⚠️ Note: Due to how the API works,testingtfm3.wordpress.comis not suggested (see Known issues above)
- Expect To not see
- Continue typing until the query becomes
{subdomain}.wordpress- Expect
testingtfm3.wordpress.comto be suggested first & available
- Expect
- Continue typing until the query becomes
{subdomain}.wordpress.com- Expect
testingtfm3.wordpress.comto be suggested and available lower in the list (scroll down if needed).
- Expect
Preview:
https://user-images.githubusercontent.com/4588074/182846402-47498ba2-49fb-42cd-a881-2aa0702bf55b.mp4
Regression Notes
-
Potential unintended areas of impact N/a - The changed code is only used for the functionality where the issue was reported.
-
What I did to test those areas of impact (or what existing automated tests I relied on) I ran the unit tests in
SiteCreationDomainSanitizerTest. I also tested manually with a bunch of variations, also tested the new code in interactive/ REPL mode with many variations. -
What automated tests I added (or what prevented me from doing so) Added new unit tests in
SiteCreationDomainSanitizerTestto verify the updated sanitization algorithm.
PR submission checklist:
- [x] I have completed the Regression Notes.
- [x] I have considered adding accessibility improvements for my changes.
- [x] I have considered if this change warrants user-facing release notes and have added them to
RELEASE-NOTES.txtif necessary.
| Warnings | |
|---|---|
| :warning: | This PR is assigned to a milestone which is closing in less than 4 days Please, make sure to get it merged by then or assign it to a later expiring milestone |
Generated by :no_entry_sign: dangerJS
You can test the WordPress changes on this Pull Request by downloading an installable build (wordpress-installable-build-pr17001-f7d31f2.apk), or scanning this QR code:
You can test the Jetpack changes on this Pull Request by downloading an installable build (jetpack-installable-build-pr17001-f7d31f2.apk), or scanning this QR code:
instrumented-tests CI check failed 2 consecutive times because of "allStatsDayLoad" that is flaky
Discussed this one with @antonis and we'd like to gather more feedback before proceeding, probably moving the sanitization logic to an API endpoint that we could use both on Android and iOS without duplicating code.
Update: internal ref: pbArwn-58N-p2 (RFC Sanitizing Domains Suggestions Queries)
This is stale, I'll remove it.