Skip to content

[PM-34124] refactor: Generalize CameraPreview analyzer parameter#6719

Open
SaintPatrck wants to merge 1 commit intomainfrom
card-scanner/1-generalize-camera-preview
Open

[PM-34124] refactor: Generalize CameraPreview analyzer parameter#6719
SaintPatrck wants to merge 1 commit intomainfrom
card-scanner/1-generalize-camera-preview

Conversation

@SaintPatrck
Copy link
Copy Markdown
Contributor

@SaintPatrck SaintPatrck commented Mar 25, 2026

🎟️ Tracking

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

📔 Objective

Generalizes the CameraPreview composable to accept any ImageAnalysis.Analyzer instead of the QR-code-specific QrCodeAnalyzer. This prepares the component for reuse by the upcoming card scanner feature while maintaining full backward compatibility with existing QR code scanning call sites.

@SaintPatrck
[PM-34124] refactor: Generalize CameraPreview analyzer parameter
c04ac3b 
Widen CameraPreview parameter from QrCodeAnalyzer to
ImageAnalysis.Analyzer to support reuse by card scanning.
Adapt both QrCodeScanScreen call sites accordingly.
@SaintPatrck SaintPatrck added the ai-review Request a Claude code review label Mar 25, 2026
@github-actions github-actions bot added app:password-manager Bitwarden Password Manager app context app:authenticator Bitwarden Authenticator app context t:tech-debt Change Type - Tech debt labels Mar 25, 2026
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Mar 25, 2026 •
edited
Loading

Claude finished @SaintPatrck's task in 1m 26s —— View job

Code Review: PR #6719

  • Gather context and read changed files
  • Review changes against Bitwarden standards
  • Post review findings

Summary

This PR cleanly generalizes the CameraPreview composable by widening the parameter type from QrCodeAnalyzer to ImageAnalysis.Analyzer and renaming the parameter from qrCodeAnalyzer to imageAnalyzer. No issues found.

Files reviewed (3):

  • ui/.../components/camera/CameraPreview.kt — Parameter type widened, renamed, unused import removed
  • app/.../qrcodescan/QrCodeScanScreen.kt — Call site updated with new parameter name
  • authenticator/.../qrcodescan/QrCodeScanScreen.kt — Call site updated with new parameter name

Verdict: The type widening is safe since QrCodeAnalyzer already extends ImageAnalysis.Analyzer. Both call sites are updated correctly. No security, correctness, or performance concerns. LGTM! ✅

@SaintPatrck SaintPatrck marked this pull request as ready for review March 25, 2026 20:45
@SaintPatrck SaintPatrck requested review from a team and david-livefront as code owners March 25, 2026 20:45
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 25, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.72%. Comparing base (d58c82c) to head (c04ac3b).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #6719      +/-   ##
==========================================
+ Coverage   85.63%   85.72%   +0.08%     
==========================================
  Files         926      944      +18     
  Lines       60201    60660     +459     
  Branches     8519     8566      +47     
==========================================
+ Hits        51555    51998     +443     
+ Misses       5683     5682       -1     
- Partials     2963     2980      +17
Flag Coverage Δ
app-data 17.77% <0.00%> (+0.09%) ⬆️
app-ui-auth-tools 20.80% <0.00%> (-0.10%) ⬇️
app-ui-platform 16.00% <0.00%> (+0.41%) ⬆️
app-ui-vault 26.73% <50.00%> (-0.13%) ⬇️
authenticator 6.50% <50.00%> (-0.04%) ⬇️
lib-core-network-bridge 4.23% <0.00%> (-0.02%) ⬇️
lib-data-ui 0.94% <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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 25, 2026

Logo
Checkmarx One – Scan Summary & Details – f23a8894-5f86-456b-b656-c4ddaf7670fc

Great job! No new security vulnerabilities introduced in this pull request

aj-rosado
aj-rosado approved these changes Mar 26, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@aj-rosado aj-rosado left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@SaintPatrck SaintPatrck added this pull request to the merge queue Mar 27, 2026
@SaintPatrck SaintPatrck removed this pull request from the merge queue due to a manual request Mar 27, 2026
@SaintPatrck SaintPatrck added innovation Feature work related to Innovation Sprint or BEEEP projects hold do not merge yet labels Mar 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aj-rosado aj-rosado aj-rosado approved these changes

@david-livefront david-livefront david-livefront approved these changes

Assignees

No one assigned

Labels

ai-review Request a Claude code review app:authenticator Bitwarden Authenticator app context app:password-manager Bitwarden Password Manager app context hold do not merge yet innovation Feature work related to Innovation Sprint or BEEEP projects t:tech-debt Change Type - Tech debt

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@SaintPatrck @aj-rosado @david-livefront