Route the remaining WP.com clients through WpComApiClientProvider - #23299
Merged
Conversation
DataViewViewModel and AddSubscribersViewModel constructed WpComApiClient directly, so they missed the locale provider and the redacting error logger that #23295 added to WpComApiClientProvider. Their WP.com responses came back unlocalized and their request failures were logged unredacted. They bypassed the provider because they need the network-request tracking interceptor, which the provider's client did not support. Give the provider an optional interceptors parameter that installs them on the OkHttp client, so these callers keep request inspection and additionally pick up the shared timeouts, error logger and language provider. DataViewViewModel now takes the provider instead of a raw WpNetworkAvailabilityProvider, which its three subclasses pass through. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
Generated by 🚫 Danger |
Contributor
|
|
Contributor
|
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## trunk #23299 +/- ##
==========================================
- Coverage 38.02% 38.02% -0.01%
==========================================
Files 2349 2349
Lines 128768 128766 -2
Branches 17971 17972 +1
==========================================
- Hits 48967 48965 -2
+ Misses 75760 75759 -1
- Partials 4041 4042 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
addSubscribers() wrapped a body that already returns Result in runCatching, yielding Result<Result<Boolean>>. The isSuccess check in onSubmitClick inspected the outer Result, which succeeds whenever nothing throws, so a WpError response or a 200 with uploadId == 0 still showed the success toast and invoked onSuccess(). Flatten with getOrElse so thrown exceptions land in the same failure channel as the inner branches, and the existing failure toast is shown. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
adalpari
marked this pull request as ready for review
September 7, 2026 09:44
adalpari
enabled auto-merge (squash)
September 9, 2026 07:51
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
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.


#TL:DR
Follow-up to #23295: the last two WP.com clients that bypassed
WpComApiClientProvidernow go through it, so they get the locale provider andthe redacting error logger too.
Description
Follow-up to #23295 (wordpress-rs 0.8.0 migration). That PR added two things
to
WpComApiClientProvider:WpComLanguageProvider, so WP.com responses come back localized (0.8.0 movedthe locale query parameter off the endpoint params and onto the client), and
WpRequestErrorLoggerusing the library's default redaction policy, replacingthe removed
WpRequestResult.toLogErrorString().Review of that PR flagged that two callers construct
WpComApiClientdirectlyrather than going through the provider, so they got neither:
DataViewViewModel(and therefore its subclassesApplicationPasswordsViewModel,SubscribersViewModel,TermsViewModel)AddSubscribersViewModelTheir WP.com responses were unlocalized, and their request failures were logged
without redaction.
Why they bypassed the provider
Both need the network-request tracking interceptor (the Chucker-based inspector),
and the provider's client didn't support interceptors — it built its own OkHttp
client with the shared wp-rs timeouts.
So rather than drop the interceptor, this PR gives the provider an optional
interceptorsparameter that installs them on that OkHttp client:Existing callers are unaffected (they pass nothing). The two former bypassers keep
request inspection and additionally gain the shared timeouts, the error logger and
the language provider.
The interceptor stays opt-in rather than being injected into the provider itself,
so this doesn't silently enable network-request tracking for every other WP.com
consumer (stats, domains, support).
Knock-on change
DataViewViewModelnow takesWpComApiClientProviderin place of the rawWpNetworkAvailabilityProviderit only used to build the client; its threesubclasses pass it through. Tests updated accordingly.
Testing instructions
./gradlew assembleJetpackDebugsucceeds./gradlew detektpassesDataViewViewModelTest,TermsViewModelTest,SubscribersViewModelTest,ApplicationPasswordsViewModelTestpassApplication Passwords, and Taxonomies/Terms.
client-level locale is now being sent
screens.
Related: follow-up to #23295
🤖 Generated with Claude Code