WordPress-Android icon indicating copy to clipboard operation
WordPress-Android copied to clipboard

Match Api domain sanitization algorithm to avoid listing available domains as unavailable

Open ovitrif opened this issue 3 years ago • 5 comments

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.w to testingtfm3.wordpres will not list testingtfm3.wordpres.com in the search results. Ideally this would work intuitively, listing the testingtfm3.wordpres.com match closer to the top as we're adding more characters for the domain name.
  • Searching for testingtfm3.wordpress.com will not show it as the first result, while searching for testingtfm3.wordpress does.
  • Searching for testingtfm3.wordpres.co shows the first result as testingtfm3.wordpres.co.wordpres.com. This feels a tad off to me but out of scope.

To test:

  1. Open Jetpack App from PR (QR-code or local build)
  2. Start the Site Creation flow
  3. Skip until the the Choose a domain screen is shown
  4. Search for a domain name (E.g. testingtfm3)
    • Expect testingtfm3ord.wordpress.com to be listed as available (scroll down if needed)
  5. Continue typing more letters of the wordpress (E.g. testingtfm3.w..testingtfm3.wordpres)
    • Expect To not see testingtfm3.wordpress.com as first result flagged as unavailable ⚠️ Note: Due to how the API works, testingtfm3.wordpress.com is not suggested (see Known issues above)
  6. Continue typing until the query becomes {subdomain}.wordpress
    • Expect testingtfm3.wordpress.com to be suggested first & available
  7. Continue typing until the query becomes {subdomain}.wordpress.com
    • Expect testingtfm3.wordpress.com to be suggested and available lower in the list (scroll down if needed).

Preview:

https://user-images.githubusercontent.com/4588074/182846402-47498ba2-49fb-42cd-a881-2aa0702bf55b.mp4

Regression Notes

  1. Potential unintended areas of impact N/a - The changed code is only used for the functionality where the issue was reported.

  2. 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.

  3. What automated tests I added (or what prevented me from doing so) Added new unit tests in SiteCreationDomainSanitizerTest to 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.txt if necessary.

ovitrif avatar Aug 04 '22 12:08 ovitrif

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:

wpmobilebot avatar Aug 04 '22 12:08 wpmobilebot

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:

wpmobilebot avatar Aug 04 '22 12:08 wpmobilebot

instrumented-tests CI check failed 2 consecutive times because of "allStatsDayLoad" that is flaky

ovitrif avatar Aug 04 '22 14:08 ovitrif

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)

ovitrif avatar Aug 05 '22 13:08 ovitrif

This is stale, I'll remove it.

ovitrif avatar Apr 20 '23 20:04 ovitrif