Fix media selection in Aztec editor

[nbradbury]

As noted in this comment, this PR broke media selection in the Aztec editor. This PR restores it and updates the related tests.

@dcalhoun I've not added testing steps since you spotted the problem and are familiar with how to test it.

[dangermattic]

	1 Warning
⚠️	PR is not assigned to a milestone.

Generated by 🚫 Danger

[wpmobilebot]

📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.

	App Name	Jetpack
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr22442-4883efd
	Commit	4883efd
	Direct Download	jetpack-prototype-build-pr22442-4883efd.apk

Note: Google Login is not supported on these builds.

[wpmobilebot]

📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.

	App Name	WordPress
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr22442-4883efd
	Commit	4883efd
	Direct Download	wordpress-prototype-build-pr22442-4883efd.apk

Note: Google Login is not supported on these builds.

[nbradbury]

@claude

[codecov]

Codecov Report

❌ Patch coverage is 50.00000% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.21%. Comparing base (7b29d97) to head (4883efd).

Files with missing lines	Patch %	Lines
...ress/android/ui/posts/editor/EditorCameraHelper.kt	0.00%	9 Missing ⚠️
...press/android/ui/posts/editor/EditorPhotoPicker.kt	90.90%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##            trunk   #22442      +/-   ##
==========================================
+ Coverage   39.16%   39.21%   +0.04%     
==========================================
  Files        2202     2203       +1     
  Lines      106560   106578      +18     
  Branches    15119    15123       +4     
==========================================
+ Hits        41734    41794      +60     
+ Misses      61328    61273      -55     
- Partials     3498     3511      +13     

☔ 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.

[dcalhoun]

@nbradbury when testing the prototype build from this PR, the first two of the three options in Aztec do not work for me; the second option causes the editor to crash.

Aztec media crash recording

Screen_Recording_20251219_082405_Jetpack.Pre-Alpha.mp4

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
6.0% Duplication on New Code

See analysis details on SonarQube Cloud

[nbradbury]

@claude

[claude]

Claude finished @nbradbury's task in 2m 16s —— View job

---

Code Review Complete

• Read and understand all changed files
• Review code quality and implementation
• Analyze security considerations
• Check performance implications
• Review test coverage and quality
• Investigate the crash issue mentioned by @dcalhoun
• Provide comprehensive feedback

---

Summary

This PR successfully restores media selection functionality in the Aztec editor by re-implementing onMediaToolbarButtonClicked() and adding a new EditorCameraHelper to centralize camera permission handling. The changes are well-structured and the tests are comprehensive.

However, I've identified a critical inconsistency that likely explains the crash @dcalhoun experienced.

---

Critical Issue: Inconsistent Camera Permission Handling

Location: EditPostActivity.kt:3228-3234 and GutenbergKitActivity.kt:2798-2804

Problem: The onCapturePhotoClicked() method calls launchCamera() directly without checking permissions:

override fun onCapturePhotoClicked() {
    if (WPMediaUtils.currentUserCanUploadMedia(siteModel)) {
        launchCamera()  // ❌ Missing permission check!
    } else {
        editorPhotoPicker?.showNoUploadPermissionSnackbar()
    }
}

Correct Implementation: The onMediaToolbarButtonClicked() method properly uses the permission check:

MediaToolbarAction.CAMERA -> {
    if (WPMediaUtils.currentUserCanUploadMedia(siteModel)) {
        editorMediaActions.checkCameraPermissionAndLaunch()  // ✅ Proper permission handling
    } else {
        showNoUploadPermissionSnackbar()
    }
}

Impact: When camera permissions aren't granted, calling launchCamera() directly will attempt to launch the camera without requesting permissions, likely causing a crash or failure.

Recommendation: Change onCapturePhotoClicked() in both activities:

override fun onCapturePhotoClicked() {
    if (WPMediaUtils.currentUserCanUploadMedia(siteModel)) {
        checkCameraPermissionAndLaunch()  // Use the permission-aware method
    } else {
        editorPhotoPicker?.showNoUploadPermissionSnackbar()
    }
}

---

Code Quality Issues

1. Line Length Violation (EditorCameraHelper.kt:21-24)

Line 120+ characters (violates project's 120-character limit from CLAUDE.md):

if (PermissionUtils.checkAndRequestCameraAndStoragePermissions(
        activity,
        WPPermissionUtils.AZTEC_EDITOR_CAMERA_PERMISSION_REQUEST_CODE
    )

This is acceptable formatting for long function calls.

2. Documentation Quality

The EditorCameraHelper is well-documented with clear KDoc comments explaining the purpose and usage. Good work!

3. Naming Consistency

The interface name EditorMediaActions is appropriate and follows the project's naming conventions.

---

Security Considerations

✅ Good:

• Proper permission handling flow with checkAndRequestCameraAndStoragePermissions()
• Consistent use of permission request codes
• Tracking of permission results in WPPermissionUtils

⚠️ Concern:

• The inconsistency in onCapturePhotoClicked() could lead to crashes on devices without camera permissions, potentially exposing error states to users

---

Performance Implications

✅ Good:

• Minimal performance impact
• Helper object pattern is efficient (object singleton)
• Lambda callbacks are appropriate here

Note: The EditorCameraHelper uses an object (Kotlin singleton), which is efficient for this stateless helper pattern.

---

Test Coverage Analysis

✅ Excellent test coverage in EditorPhotoPickerTest.kt:

1. Tests added for onMediaToolbarButtonClicked() (lines 326-388):

   • Null action handling
   • Camera launch with upload permission
   • Camera blocked without upload permission
   • Library launch
   • Gallery display with fragment manager mocking

2. Tests for onIconClicked() already existed (lines 200-322):

   • Properly test OpenCameraForPhotos action
   • Test permission checks
   • Test all data source switches

⚠️ Missing Coverage:

• No tests for EditorCameraHelper itself
• No tests for the permission result handling in handlePermissionResult()
• onCapturePhotoClicked() is not tested (and has the bug mentioned above)

Recommendation: Consider adding unit tests for EditorCameraHelper:

@Test
fun `checkCameraPermissionAndLaunch calls launchCamera when permissions granted`() {
    // Test the helper methods directly
}

@Test
fun `handlePermissionResult returns true for correct request code`() {
    // Test the permission result handling
}

---

Architecture & Design

✅ Excellent refactoring:

• Extracting EditorCameraHelper eliminates duplication between EditPostActivity and GutenbergKitActivity
• The EditorMediaActions interface provides clear separation of concerns
• The callback-based design is clean and testable

Minor Suggestion: Consider adding @VisibleForTesting annotation if this helper is primarily for internal use.

---

Additional Observations

EditorPhotoPicker.kt:125

The conditional camera setup is correct:

cameraSetup = if (showAztecEditor) CameraSetup.HIDDEN else CameraSetup.ENABLED,

This properly hides the camera FAB in Aztec (since Aztec has toolbar buttons) and shows it in Gutenberg.

WPPermissionUtils.java:38

Good addition of the permission request code constant:

public static final int AZTEC_EDITOR_CAMERA_PERMISSION_REQUEST_CODE = 42;

Properly documented that these codes are reported to analytics.

---

Recommendations Summary

1. 🔴 Critical: Fix onCapturePhotoClicked() in both activities to use checkCameraPermissionAndLaunch()
2. 🟡 Medium: Add unit tests for EditorCameraHelper
3. 🟢 Nice-to-have: Consider adding @VisibleForTesting annotations

---

Final Verdict

The PR successfully restores Aztec media selection functionality with well-structured code and good test coverage. However, the critical inconsistency in onCapturePhotoClicked() needs to be addressed before merging, as it likely explains the crash @dcalhoun experienced.

Once the camera permission handling is made consistent across all code paths, this will be a solid improvement to the codebase.

---

| Branch: issue/fix-aztec-photo-picker