[SAAS-19868] Add face capture widget#3780
Conversation
📝 WalkthroughWalkthroughThis PR introduces Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
avazirna
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/org/commcare/views/widgets/ImageWidget.java (1)
206-216: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the discard action available when preview reconstruction fails.
mBinaryNamecan already be set whiletoDisplayis missing (for example after cleanup or decryption/display-file issues). WithmDiscardButton.setVisibility(View.VISIBLE)gated bytoDisplay.exists(), the widget shows a stale answer but removes the normal way to clear it.Suggested fix
if (toDisplay.exists()) { Bitmap bmp = MediaUtil.getBitmapScaledToContainer(toDisplay, screenHeight, screenWidth); if (bmp == null) { mErrorTextView.setVisibility(View.VISIBLE); } mImageView.setImageBitmap(bmp); - mDiscardButton.setVisibility(View.VISIBLE); } else { mImageView.setImageBitmap(null); } + mDiscardButton.setVisibility(View.VISIBLE);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/org/commcare/views/widgets/ImageWidget.java` around lines 206 - 216, The discard button visibility in ImageWidget’s preview update flow is incorrectly tied to the reconstructed file existing, which hides the user’s only way to clear a stale binary when `mBinaryName` is still set but `toDisplay` is missing. Update the logic around the `toDisplay.exists()` check in `ImageWidget` so `mDiscardButton.setVisibility(View.VISIBLE)` still runs whenever an attachment is present, even if `MediaUtil.getBitmapScaledToContainer()` returns null or the file is absent, while keeping the image preview handling separate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/org/commcare/fragments/MicroImageActivity.java`:
- Around line 421-423: The capture output mode parsing in getCaptureOutputMode()
can still crash on unexpected intent values because
CaptureOutputMode.valueOf(...) throws for unknown strings. Update
MicroImageActivity’s getCaptureOutputMode() to treat invalid or stale
CAPTURE_OUTPUT_MODE_EXTRA values the same as a missing extra by falling back to
DEFAULT_CAPTURE_OUTPUT_MODE, using the existing CaptureOutputMode and
CAPTURE_OUTPUT_MODE_EXTRA symbols to locate the logic.
- Around line 138-142: Gate the camera lens switch UI and initial camera binding
on actual lens availability. In MicroImageActivity, use
ProcessCameraProvider.hasCamera(...) before the first bindToLifecycle(...) call
to verify the selected CameraSelector exists, and only show/enable the
switchCameraLensButton when the alternate lens is present. Keep
getAllowCameraLensSwitch() as the feature flag, but prevent exposing a selector
that the device cannot satisfy.
---
Outside diff comments:
In `@app/src/org/commcare/views/widgets/ImageWidget.java`:
- Around line 206-216: The discard button visibility in ImageWidget’s preview
update flow is incorrectly tied to the reconstructed file existing, which hides
the user’s only way to clear a stale binary when `mBinaryName` is still set but
`toDisplay` is missing. Update the logic around the `toDisplay.exists()` check
in `ImageWidget` so `mDiscardButton.setVisibility(View.VISIBLE)` still runs
whenever an attachment is present, even if
`MediaUtil.getBitmapScaledToContainer()` returns null or the file is absent,
while keeping the image preview handling separate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 38330e7d-a48e-4049-9157-3942b520a27e
📒 Files selected for processing (14)
app/assets/locales/android_translatable_strings.txtapp/res/color/face_capture_indicator_selector.xmlapp/res/drawable/capture_mode_indicator.xmlapp/res/drawable/ic_camera_shutter.xmlapp/res/drawable/ic_switch_camera_lens.xmlapp/res/drawable/instruction_hand.xmlapp/res/layout/micro_image_widget.xmlapp/res/values/strings.xmlapp/src/org/commcare/activities/FormEntryActivity.javaapp/src/org/commcare/activities/components/FormEntryConstants.javaapp/src/org/commcare/fragments/MicroImageActivity.javaapp/src/org/commcare/views/widgets/FaceCaptureWidget.ktapp/src/org/commcare/views/widgets/ImageWidget.javaapp/src/org/commcare/views/widgets/WidgetFactory.java
| if (getAllowCameraLensSwitch()) { | ||
| cameraControlsContainer.setVisibility(VISIBLE); | ||
| switchCameraLensButton.setVisibility(VISIBLE); | ||
| switchCameraLensButton.setOnClickListener(v -> switchCameraLensFacing()); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Gate camera selection on available lenses.
ALLOW_CAMERA_LENS_SWITCH_EXTRA can expose a selector the device doesn’t support, and bindToLifecycle(...) will fail when the chosen CameraSelector has no match. Check ProcessCameraProvider.hasCamera(...) before the initial bind and disable the switch when the alternate lens is missing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/org/commcare/fragments/MicroImageActivity.java` around lines 138 -
142, Gate the camera lens switch UI and initial camera binding on actual lens
availability. In MicroImageActivity, use ProcessCameraProvider.hasCamera(...)
before the first bindToLifecycle(...) call to verify the selected CameraSelector
exists, and only show/enable the switchCameraLensButton when the alternate lens
is present. Keep getAllowCameraLensSwitch() as the feature flag, but prevent
exposing a selector that the device cannot satisfy.
| private CaptureOutputMode getCaptureOutputMode() { | ||
| return CaptureOutputMode.valueOf(Objects.requireNonNullElse(getIntent().getStringExtra(CAPTURE_OUTPUT_MODE_EXTRA), DEFAULT_CAPTURE_OUTPUT_MODE)); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Fall back on unknown capture-mode values instead of crashing.
CaptureOutputMode.valueOf(...) throws on any unexpected string. Since this activity already has a default for a missing extra, stale or mistyped callers should fall back to DEFAULT_CAPTURE_OUTPUT_MODE here too rather than crashing before the image is returned.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/org/commcare/fragments/MicroImageActivity.java` around lines 421 -
423, The capture output mode parsing in getCaptureOutputMode() can still crash
on unexpected intent values because CaptureOutputMode.valueOf(...) throws for
unknown strings. Update MicroImageActivity’s getCaptureOutputMode() to treat
invalid or stale CAPTURE_OUTPUT_MODE_EXTRA values the same as a missing extra by
falling back to DEFAULT_CAPTURE_OUTPUT_MODE, using the existing
CaptureOutputMode and CAPTURE_OUTPUT_MODE_EXTRA symbols to locate the logic.
# Conflicts: # app/src/org/commcare/activities/camera/MicroImageActivity.java
� Conflicts: � app/src/org/commcare/activities/camera/MicroImageActivity.java � app/src/org/commcare/views/widgets/FaceCaptureWidget.kt
avazirna
force-pushed
the
saas-19868_create-face-capture-widget
branch
from
97892b5 to
07c70bd
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3780 +/- ##
============================================
- Coverage 26.38% 26.37% -0.02%
- Complexity 4522 4526 +4
============================================
Files 969 971 +2
Lines 57973 58025 +52
Branches 6898 6904 +6
============================================
+ Hits 15296 15303 +7
- Misses 40802 40848 +46
+ Partials 1875 1874 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
avazirna
force-pushed
the
saas-19868_create-face-capture-widget
branch
from
07c70bd to
d139a1f
Compare
avazirna
force-pushed
the
saas-19868_create-face-capture-widget
branch
from
544445b to
f7c0649
Compare
avazirna
force-pushed
the
saas-19868_create-face-capture-widget
branch
from
4298152 to
be79650
Compare
avazirna
requested review from
Jignesh-dimagi,
OrangeAndGreen,
conroy-ricketts and
shubham1g5
1 participant
Product Description
Adds a dedicated face capture widget for
<upload mediatype="image/*" appearance="face">questions. When a user reaches such a question, CommCare opens a guided face-capture screen instead of the standard image widget:face_capture_auto_mode.mp4
face_capture_manual_mode.mp4
Technical Summary
Ticket: SAAS-19868
Face capture is selected purely from the
appearanceattribute on image-upload questions, so this PR is self-contained on the client side:WidgetFactoryroutes theCONTROL_IMAGE_CHOOSEcase toFaceCaptureWidgetwhenappearance="face"(alongside the existingappearance="signature"→SignatureWidget), defaulting toImageWidgetfor null or unrecognized appearances.FaceCaptureWidgetlaunchesMicroImageActivityand delivers the result to a temp file for form submission; wired throughWidgetFactory,ImageWidget,FormEntryActivity, andFormEntryConstants.MicroImageActivitygains capture-mode handling (auto/manual), front/back lens switching gated by theALLOW_CAMERA_LENS_SWITCH_EXTRAintent extra, plus the instruction and mode-indicator UI.Safety Assurance
Safety story
appearance="face"; all other upload questions (plainimage/*,signature, audio, video, document) keep their current behavior.MicroImageActivitycapture/output pipeline and add a route to save the image to a temp folder for processing;Automated test coverage
No automated coverage added — the change is UI/camera-centric (CameraX + ML Kit), which isn't exercised by the current instrumentation suite. Behavior was validated as described above.
Labels and Review
🤖 Generated with Claude Code