[PM-37255] feat: Consume fill-assist targeting rules data#6990
[PM-37255] feat: Consume fill-assist targeting rules data#6990aj-rosado wants to merge 2 commits into
Conversation
github-actions
Bot
added
app:password-manager
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE This PR adds the network layer for consuming fill-assist targeting rules from the Bitwarden fill-assist CDN. Changes include a new Code Review DetailsNo findings. Notes on items considered and dismissed:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6990 +/- ##
==========================================
+ Coverage 85.63% 86.47% +0.84%
==========================================
Files 1004 873 -131
Lines 66223 63670 -2553
Branches 9299 9234 -65
==========================================
- Hits 56710 55059 -1651
+ Misses 6321 5436 -885
+ Partials 3192 3175 -17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| val schemaVersion: String? = null, | ||
|
|
||
| @SerialName("hosts") | ||
| val hosts: Map<String, HostEntryJson?>? = null, |
There was a problem hiding this comment.
Why do we have all the default values here?
Also, is everything in this model really nullable?
| val gitSha: String? = null, | ||
|
|
||
| @SerialName("maps") | ||
| val maps: MapsJson? = null, |
There was a problem hiding this comment.
Same questions as above
|
|
||
| class FillAssistFormsJsonTest { | ||
|
|
||
| private val json = Json { ignoreUnknownKeys = true } |
There was a problem hiding this comment.
Can we use the real Json instance to insure the tests match reality?
private val json = CoreModule.providesJson(buildInfoManager = mockk(relaxed = true))There was a problem hiding this comment.
Hmmm, looking closer, do these tests just verify that the objects serialize properly?
We normally do not write tests for that as the FillAssistServiceTest should cover that for us
2 participants
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-37255
📔 Objective
Changes:
ConfigResponseJson.EnvironmentJson— addsfillAssistRulesUrl: String?(@SerialName("fillAssistRules")) from the server config responseFillAssistManifestJson— model for the CDN manifest file, with a version-keyed map (Map<String, FileEntryJson?>) so new schema versions appear automatically without model changesFillAssistFormsJson— model for the forms rules file; usesJsonElementfor the composite selector array to handle both string and array-of-string alternativesFillAssistApi/FillAssistService/FillAssistServiceImpl— Retrofit service using@Urlannotation withcreateStaticRetrofit()to bypassBaseUrlInterceptor(the fill-assist CDN is external to the Bitwarden API)BitwardenServiceClient— exposesfillAssistServicePlatformNetworkModule— providesFillAssistServiceEnvironmentJsontest fixtures updated withfillAssistRulesUrl = null