Skip to content

[PM-37255] feat: Consume fill-assist targeting rules data#6990

Open
aj-rosado wants to merge 2 commits into
mainfrom
PM-37255/fill-assist-network-layer
Open

[PM-37255] feat: Consume fill-assist targeting rules data#6990
aj-rosado wants to merge 2 commits into
mainfrom
PM-37255/fill-assist-network-layer

Conversation

@aj-rosado
Copy link
Copy Markdown
Contributor

@aj-rosado aj-rosado commented May 29, 2026

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-37255

📔 Objective

Changes:

  • ConfigResponseJson.EnvironmentJson — adds fillAssistRulesUrl: String? (@SerialName("fillAssistRules")) from the server config response
  • FillAssistManifestJson — model for the CDN manifest file, with a version-keyed map (Map<String, FileEntryJson?>) so new schema versions appear automatically without model changes
  • FillAssistFormsJson — model for the forms rules file; uses JsonElement for the composite selector array to handle both string and array-of-string alternatives
  • FillAssistApi / FillAssistService / FillAssistServiceImpl — Retrofit service using @Url annotation with createStaticRetrofit() to bypass BaseUrlInterceptor (the fill-assist CDN is external to the Bitwarden API)
  • BitwardenServiceClient — exposes fillAssistService
  • PlatformNetworkModule — provides FillAssistService
  • All EnvironmentJson test fixtures updated with fillAssistRulesUrl = null

@aj-rosado aj-rosado added the ai-review Request a Claude code review label May 29, 2026
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context app:authenticator Bitwarden Authenticator app context t:feature Change Type - Feature Development labels May 29, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 29, 2026

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR adds the network layer for consuming fill-assist targeting rules from the Bitwarden fill-assist CDN. Changes include a new FillAssistService (interface + impl) with two endpoints (manifest and forms), corresponding JSON models with ignoreUnknownKeys-tolerant deserialization, a fillAssistRulesUrl field on EnvironmentJson (additive, backward-compatible), and Hilt wiring via PlatformNetworkModule. The code consistently follows existing service patterns (e.g., DownloadServiceImpl), uses createStaticRetrofit() with @Url to bypass BaseUrlInterceptor for the external CDN, and all EnvironmentJson test fixtures across both apps and the data/network modules have been updated. The previously flagged unrelated PolicyManager mock change in AuthRepositoryTest was reverted in f57a7d09a, leaving only the necessary fillAssistRulesUrl = null addition.

Code Review Details

No findings.

Notes on items considered and dismissed:

  • FillAssistManifestJson.MapsJson.forms lacks a = null default unlike sibling nullable properties — harmless under the project's explicitNulls = false JSON config; pure consistency nit.
  • getManifest(url:) vs getForms(formsUrl:) parameter naming differs slightly — cosmetic.
  • New JSON model tests use a local Json { ignoreUnknownKeys = true } rather than CoreModule.providesJson(...) — acceptable for pure model decoding tests since contextual serializers and explicitNulls aren't exercised.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.47%. Comparing base (e7e2c26) to head (f57a7d0).
⚠️ Report is 2 commits behind head on main.

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     
Flag Coverage Δ
app-data 17.23% <0.00%> (+0.19%) ⬆️
app-ui-auth-tools 19.02% <0.00%> (-0.18%) ⬇️
app-ui-platform 16.49% <0.00%> (-0.56%) ⬇️
app-ui-vault 27.81% <0.00%> (-0.57%) ⬇️
authenticator 6.21% <0.00%> (-0.05%) ⬇️
lib-core-network-bridge 4.11% <100.00%> (-0.01%) ⬇️
lib-data-ui 1.15% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@aj-rosado aj-rosado marked this pull request as ready for review May 29, 2026 14:20
@aj-rosado aj-rosado requested review from a team and david-livefront as code owners May 29, 2026 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review app:authenticator Bitwarden Authenticator app context app:password-manager Bitwarden Password Manager app context t:feature Change Type - Feature Development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant