CMM-1054 when opening an unauthorized post, the three dots menu icon is not working

[adalpari]

Description

Problem

When opening a deeplink to a Reader post the user isn't authorized to view:

• The error message said "Use the top action button to try a browser instead"
• The three-dots menu was visible but did nothing when tapped (
• The browse icon was hidden

Solution

• Hide all toolbar menu items in error state (they require a loaded post to function)
• Add "Open in browser" button below the error message when opened via deeplink
• Update error message strings to remove references to the non-existent "top action button"

Testing instructions

1. While being unlogged to WP.COM, spen a deeplink to a private/restricted P2

• Verify error message displays with "Open in browser" button (no toolbar menu icons), and it opens the browser

3. log into WP.COM, and open a link to a P2 you are not part of

• Verify error message displays with "Open in browser" button (no toolbar menu icons), and it opens the browser

• Open other posts form the reader or from deeplinks and verify the top menu is shown and works

Before:

After:

[dangermattic]

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

Generated by 🚫 Danger

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

[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	pr22412-a94b641
	Commit	a94b641
	Direct Download	jetpack-prototype-build-pr22412-a94b641.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	pr22412-a94b641
	Commit	a94b641
	Direct Download	wordpress-prototype-build-pr22412-a94b641.apk

Note: Google Login is not supported on these builds.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 39.09%. Comparing base (edb1fc8) to head (a94b641).

Additional details and impacted files

@@           Coverage Diff           @@
##            trunk   #22412   +/-   ##
=======================================
  Coverage   39.09%   39.09%           
=======================================
  Files        2207     2207           
  Lines      106858   106860    +2     
  Branches    15159    15160    +1     
=======================================
+ Hits        41776    41778    +2     
  Misses      61566    61566           
  Partials     3516     3516           

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

[adalpari]

@claude

[claude]

Claude finished @adalpari's task in 1m 40s —— View job

---

Code Review Complete ✅

I've reviewed the PR changes for fixing CMM-1054. This is a clean, well-implemented bug fix that addresses the issue where the three-dots menu was visible but non-functional in the error state.

---

Summary

What Changed:

• Hides all toolbar menu items when an unauthorized post error occurs
• Adds a new "Open in browser" CTA button that appears below the error message when opened via deeplink
• Updates error message strings to remove references to the non-existent "top action button"

Files Changed:

• ReaderPostDetailFragment.kt: Added hideMenu() function call and "Open in browser" button initialization
• ReaderPostDetailViewModel.kt: Added openInBrowserButtonVisibility parameter to ErrorUiState
• reader_fragment_post_detail.xml: Added new "Open in browser" button UI
• strings.xml: Removed misleading references and added new button text
• ReaderPostDetailViewModelTest.kt: Added comprehensive test coverage for the new functionality

---

Positive Observations

1. Clean separation of concerns - The error state handling is well-structured with proper visibility control in the ViewModel
2. Comprehensive test coverage - 4 new unit tests added covering all scenarios (with/without intercepted URI, with/without sign-in offer)
3. Consistent with existing patterns - The openInBrowserButton follows the same pattern as the existing nux_sign_in_button
4. User experience improvement - The solution provides a clear alternative action when users can't access a post
5. Good resource management - Proper use of string resources and no hardcoded strings

---

Code Quality Observations

ReaderPostDetailFragment.kt (1109-1114)

The hideMenu() function is straightforward and effective:

private fun hideMenu(menu: Menu) {
    menu.findItem(R.id.menu_browse)?.isVisible = false
    menu.findItem(R.id.menu_share)?.isVisible = false
    menu.findItem(R.id.menu_reading_preferences)?.isVisible = false
    menu.findItem(R.id.menu_more)?.isVisible = false
}

Observation: This function manually hides each menu item individually. Consider adding a comment explaining why all items need to be hidden (they require a loaded post to function).

ReaderPostDetailFragment.kt (503-511)

The "Open in browser" button initialization:

private fun initOpenInBrowserButton(view: View) {
    view.findViewById<View>(R.id.open_in_browser_button).setOnClickListener {
        viewModel.interceptedUri?.let { uri ->
            readerTracker.trackUri(AnalyticsTracker.Stat.DEEP_LINKED_FALLBACK, uri)
            ReaderActivityLauncher.openUrl(requireActivity(), uri, OpenUrlType.EXTERNAL)
            requireActivity().finish()
        }
    }
}

Good practices:

• Proper null safety with let
• Analytics tracking before navigation
• Finishes activity after opening browser (appropriate for deeplink fallback scenario)

ReaderPostDetailViewModel.kt (711-719)

The error state construction with proper visibility flags:

private fun trackAndUpdateNotAuthorisedErrorState() {
    trackNotAuthorisedState()
    
    _uiState.value = ErrorUiState(
        message = UiStringRes(getNotAuthorisedErrorMessageRes()),
        signInButtonVisibility = shouldOfferSignIn,
        openInBrowserButtonVisibility = interceptedUri != null
    )
}

Excellent: Clean logic - the browser button only shows when there's actually a URI to open.

Layout Changes (reader_fragment_post_detail.xml:158-171)

<org.wordpress.android.widgets.WPTextView
    android:id="@+id/open_in_browser_button"
    style="@style/WordPress.NUXPrimaryButton"
    android:layout_width="match_parent"
    android:layout_marginTop="@dimen/margin_extra_large"
    android:layout_below="@+id/text_error"
    android:clickable="true"
    android:focusable="true"
    android:gravity="center"
    android:text="@string/reader_open_in_browser"
    android:visibility="gone"
    android:layout_marginEnd="@dimen/reader_detail_margin"
    android:layout_marginStart="@dimen/reader_detail_margin"
    tools:visibility="visible"/>

Concern: Both nux_sign_in_button (line 149) and open_in_browser_button (line 162) use android:layout_below="@+id/text_error". This could cause layout issues if both buttons are visible simultaneously. However, looking at the ViewModel logic, only one should be visible at a time:

• Sign-in button: visible when shouldOfferSignIn is true
• Browser button: visible when interceptedUri != null

Question: Can both conditions be true simultaneously? If yes, the layout would need adjustment to stack the buttons properly.

---

Testing Coverage

The test coverage is comprehensive with 4 new tests covering all combinations:

ReaderPostDetailViewModelTest.kt:582-608

• ✅ Test with intercepted URI → button visible
• ✅ Test without intercepted URI → button not visible
• ✅ Tests cover both sign-in scenarios

Strong test coverage ensures the feature works correctly across all states.

---

String Resources

The string changes appropriately remove misleading references to "top action button":

- "You're not authorized to view this post. Use the top action button to try a browser instead."
+ "You're not authorized to view this post"

- "Try signing in to WordPress.com first or use the top action button to open a browser instead."
+ "Try signing in to WordPress.com first."

New string added:

<string name="reader_open_in_browser">Open in browser</string>

Good: Clear, concise, and user-friendly.

---

Minor Suggestions

1. Layout consideration: Verify that the sign-in and browser buttons never appear simultaneously. If they can, adjust the layout to stack them properly using a LinearLayout or proper RelativeLayout chaining.

2. Documentation: Consider adding a KDoc comment to hideMenu() explaining it's called in error states because menu items require a loaded post.

3. Accessibility: Both buttons have proper clickable and focusable attributes, but consider adding contentDescription attributes for screen readers.

4. Edge case: The openInBrowserButton click handler uses requireActivity().finish() - verify this behaves correctly in all navigation scenarios (e.g., when the activity is in a task stack).

---

Security Considerations

✅ No security issues identified:

• The intercepted URI is opened with OpenUrlType.EXTERNAL, which is appropriate for user-initiated browser navigation
• Proper null safety prevents crashes
• Analytics tracking doesn't expose sensitive data

---

Performance Considerations

✅ No performance concerns:

• Button initialization is lightweight
• Menu hiding is a simple visibility toggle
• No unnecessary object allocations

---

Conclusion

This is a well-implemented fix that addresses the reported issue effectively. The code follows Android best practices, maintains consistency with the existing codebase, and includes solid test coverage. The user experience improvement is clear - users now have a working alternative when they can't access a post via deeplink.

Recommendation: ✅ Approve with the minor layout verification noted above.

---

|