Migrate the Site Creation Domain Search to wordpress-rs - #23247
Open
oguzkocer wants to merge 10 commits into
Open
Migrate the Site Creation Domain Search to wordpress-rs#23247oguzkocer wants to merge 10 commits into
wordpress-rs#23247oguzkocer wants to merge 10 commits into
Conversation
… FluxC to wordpress-rs Replace FluxC's dispatcher/EventBus domain suggestions with direct wordpress-rs API calls: - `FetchDomainsUseCase` now uses `WpComApiClient.domains().suggestions()` instead of FluxC's `Dispatcher` + `OnSuggestedDomains` event - `SiteCreationDomainsViewModel` uses wordpress-rs for products fetch instead of `ProductsStore` - Handle `invalid_query` API error via `WpErrorCode.CustomException` to preserve the empty-results UX (matching FluxC's `INVALID_QUERY`) - Remove `Dispatcher` registration/unregistration from ViewModel
`quantity` is always `FETCH_DOMAINS_SIZE`, so the parameter was suppressed rather than read. No caller passed it. Changes: - Remove `size` from `FetchDomainsUseCase.fetchDomains` - Drop it from the stubs and verifications in `SiteCreationDomainsViewModelTest`
The caller passes the query in and holds it across the call, so it never read the copy coming back. Echoing it was needed when the result arrived over EventBus and had to be matched to its request. Changes: - Remove `query` from `FetchDomainsResult`, making `InvalidQuery` and `Error` data objects - Update the ViewModel branches and both test classes
Every rejection reported as a generic error with no message, and `empty_results` reached the user as a failure with a retry button rather than the empty-list message it stands for. `SuggestDomainErrorType` supplied the tracked type before, derived from the same API code; the tracker lowercases what it is given, so the code goes through as-is. Changes: - Carry the API error code and message on `FetchDomainsResult.Error` and track them instead of a hardcoded `GENERIC_ERROR` - Map `empty_results` to an empty `Success` - Log the product-fetch failure with `toLogErrorString()` - Cover the `invalid_query`, `empty_results`, and other-code paths, and assert the tracked type and message
Changes:
- Sort the `rs.wordpress` and `uniffi` imports after the `org.wordpress`
block in the ViewModel and its test, matching the other migrated classes
- Make `SALE_PRODUCTS_COUNT` a const and drop its `@Suppress("unused")`;
both sale tests read it
- Drop an `@Suppress("UNCHECKED_CAST")` from a test with no cast, and a
stray blank line
`SiteCreationDomainsViewModel` requested the products list on start and stored it in a field nothing read, so the screen paid for a request it made no use of. `NewDomainsSearchRepository` and `DomainSuggestionsViewModel` are the callers that read a product, both for `combinedSaleCostDisplay`, and neither is affected. Changes: - Remove the products fetch, the cached field, and the API client it needed, along with the `WpComApiClientProvider` and `AccountStore` dependencies - Remove the test asserting the fetch happened once, and the two `@Ignore`d sale tests it fed
`Cost.OnSale` and `Tag.Sale` were only ever built by `DomainItemPreview`, so the sale price and its tag rendered in Android Studio and nowhere else. Changes: - Remove `Cost.OnSale`, `Tag.Sale`, the `SalePrice` composable, and the branch selecting it - Remove them from the preview fixture - Remove the `site_creation_domain_cost_sale` and `site_creation_domain_tag_sale` strings they used
Typing the first character made the clear button and the spinner appear, and the `LayoutTransition` animated the neighbouring views' bounds through zero height. The framework clears focus on a view animated to zero size, so the field lost it, the toolbar's navigation button took it, and the input connection stayed inactive until the results arrived and the field asked for focus back. Every character typed in between was dropped. Both layouts belong to this screen alone. The header still animates itself in `updateHeader`; what is lost is the implicit animation of the views around it. Changes: - Drop `android:animateLayoutChanges` from `site_creation_domains_screen` and `site_creation_search_input_item`
Collaborator
Generated by 🚫 Danger |
Checkstyle rejects an empty line before a closing brace, and removing `Tag.Sale` and `Cost.OnSale` left one behind in each. Changes: - Drop the blank lines before the `Tag` and `Cost` closing braces
Contributor
|
|
Contributor
|
|
Contributor
🤖 Build Failure AnalysisThis build has failures. Claude has analyzed them - check the build annotations for details. |
Lint's `ExtraTranslation` rejects a string that a locale translates but the default locale does not define, and the two sale strings left the default locale with the UI that used them. Changes: - Delete `site_creation_domain_tag_sale` and `site_creation_domain_cost_sale` from every `values-*/strings.xml`
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #23247 +/- ##
=======================================
Coverage 37.97% 37.98%
=======================================
Files 2336 2336
Lines 127220 127218 -2
Branches 17629 17632 +3
=======================================
+ Hits 48312 48319 +7
+ Misses 74960 74951 -9
Partials 3948 3948 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
oguzkocer
marked this pull request as ready for review
August 20, 2026 22:13
oguzkocer
requested review from
a team and
adalpari
and removed request for
a team
August 20, 2026 22:13
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Moves
SiteCreationDomainsViewModelandFetchDomainsUseCaseoff FluxC and onto wordpress-rs.FetchDomainsUseCasecallsWpComApiClient.domains().suggestions()directly instead of dispatchingnewSuggestDomainsActionand waiting for anOnSuggestedDomainsevent, so the EventBus subscription, the query-to-continuation pairing, and the ViewModel's dispatcher registration all go away.Behaviour is meant to match trunk. The pieces worth checking:
empty_resultsis not a failure. The API reports "no domains for that search" as an HTTP error, and FluxC turned it back into a successful empty list. The migration does the same, so the screen shows the empty-list message rather than a red error with a retry button.invalid_querykeeps its own message. Same as trunk, and it is not tracked as an error.FetchDomainsResult.Errorcarries the API's error code and message through totrackErrorShown.SuggestDomainErrorTypesupplied that value before, derived from the same code, and the tracker lowercases whatever it is given, so the value reported for a known code is unchanged.Three cleanups came out of reviewing the migration:
fetchDomainshad asizeparameter that was suppressed rather than read, sincequantityis alwaysFETCH_DOMAINS_SIZE. No caller passed it.FetchDomainsResultechoed thequeryback, which the caller already held. That existed to match an EventBus result to its request and a suspend call needs no such thing.NewDomainsSearchRepositoryandDomainSuggestionsViewModelare the callers that read a product, both forcombinedSaleCostDisplay, and neither is affected. The sale UI this screen kept for it —Cost.OnSale,Tag.Sale, theSalePricecomposable — was only ever built byDomainItemPreview, so it rendered in Android Studio and nowhere else.Dropping
animateLayoutChangeson this screenOn trunk, typing the first character into the domain search stops you typing until that first request finishes — every keystroke in between is lost. The implicit layout animation that runs when the clear button and the spinner appear takes focus off the search field, and nothing gives it back until the results arrive.
Removing
animateLayoutChangesfrom this screen's two layouts fixes it. The trade is that the views around the header snap into place rather than sliding. It reads as snappier to me and losing keystrokes was the worse of the two, but it is a visible change.Testing instructions
Reach the screen: My Site → the site name dropdown → + → Create WordPress.com site → skip or fill the two steps before Choose a domain.
Typing is not interrupted:
A query the API refuses:
.A query with no matches:
zcharacters.A normal search:
coolsite..wordpress.comoption among them.No connection:
Screenshots