CMM-1208 strange subscription notifications UI settings

[adalpari]

Description

Fixes UI inconsistencies in the Reader blog subscription settings button behavior.

Problems:

1. The subscription settings button was not visible after subscribing, but visible after unsubscribing (inverted behavior)
2. The "notify via email" switch showed incorrect state because the settings were accessible when unsubscribed, displaying stale data
3. The settings button remained clickable during the unsubscribe operation

Root cause:
In ReaderSiteHeaderView.java, after a successful follow/unfollow action, the cached mBlogInfo.isFollowing was never updated and
updateSubscriptionSettingsButtonVisibility() was never called, causing the button visibility to remain based on stale state.

Fix:

• Update mBlogInfo.isFollowing after successful follow/unfollow
• Call updateSubscriptionSettingsButtonVisibility() to refresh the button immediately
• Disable the settings button while unsubscribing to prevent interaction during the operation
• Re-enable the button if unsubscribe fails

Testing instructions

Test subscription settings button visibility:

1. Open the Reader and navigate to a blog you are NOT subscribed to
2. Tap the Subscribe button

• Verify the subscription settings icon appears after subscribing

3. Tap the subscription settings icon

• Verify the settings bottom sheet opens with correct toggle states

4. Close the settings and tap the Subscribed button to unsubscribe

• Verify the settings icon becomes disabled while unsubscribing
• Verify the settings icon disappears after unsubscribing

Screen_recording_20260202_121059.mp4

[adalpari]

@claude

[dangermattic]

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

Generated by 🚫 Danger

[claude]

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

---

Code Review Completed ✓

• Read the changed file and understand the implementation
• Review code quality and best practices
• Check for security issues
• Analyze performance implications
• Verify testing approach
• Review documentation
• Provide comprehensive feedback

---

Summary

The implementation correctly addresses the UI inconsistencies described in the PR. The fix properly updates the cached mBlogInfo.isFollowing state and refreshes button visibility after follow/unfollow operations. The code changes are minimal, focused, and solve the specific problem without introducing unnecessary complexity.

Detailed Review

✅ Code Quality & Best Practices

Strengths:

• Clean, minimal fix that directly addresses the root cause
• Proper state management: updates cached state immediately after successful operations
• Good defensive programming with null checks (mBlogInfo != null, mSubscriptionSettingsButton != null)
• Consistent with existing code patterns in the file
• Logical separation of concerns: button state managed in dedicated method

Minor Observations:

• Line 398 (mSubscriptionSettingsButton.setEnabled(showSettings);) is good for consistency but slightly redundant since setEnabled(true) is called on a button that's already being made VISIBLE. This is defensive and acceptable, but worth noting that visibility changes typically imply interactability.

✅ Security

No security concerns identified. The changes are UI-focused state management with no:

• User input handling
• Data persistence
• Authentication/authorization logic changes
• Network calls (only callback handling)

✅ Performance

Excellent performance characteristics:

• Minimal computational overhead: simple boolean updates and visibility changes
• No additional network calls introduced
• No memory leaks: uses existing cached mBlogInfo reference
• UI updates happen on main thread (already handled by existing callback mechanism)

🟡 Testing

Concern: Missing test coverage

This file has no associated unit tests. The changes modify critical user-facing behavior (button visibility and state) but lack automated verification.

Recommendation: While not blocking for this PR (fixing a bug is valuable), consider adding tests for:

1. Subscription settings button visibility after successful follow
2. Button visibility after successful unfollow
3. Button state during unsubscribe operation (disabled)
4. Button state after failed unsubscribe (re-enabled)

Test file suggestion: Create WordPress/src/test/java/org/wordpress/android/ui/reader/views/ReaderSiteHeaderViewTest.kt

Example test cases:

@Test
fun `subscription settings button visible after successful follow`() {
    // Given: blog info with isFollowing = false
    // When: follow succeeds
    // Then: button visibility = VISIBLE
}

@Test
fun `subscription settings button hidden after successful unfollow`() {
    // Given: blog info with isFollowing = true
    // When: unfollow succeeds
    // Then: button visibility = GONE
}

@Test
fun `subscription settings button disabled during unsubscribe`() {
    // Given: subscribed blog
    // When: unsubscribe initiated
    // Then: button.isEnabled = false
}

@Test
fun `subscription settings button re-enabled if unsubscribe fails`() {
    // Given: subscribed blog
    // When: unsubscribe fails
    // Then: button.isEnabled = true
}

✅ Documentation

The PR description provides excellent documentation:

• Clear problem statement with specific symptoms
• Root cause analysis
• Solution explanation
• Comprehensive testing instructions

The code is self-explanatory with the inline comments added (lines 320, 345, 355).

Edge Cases Considered

The implementation handles key edge cases:

1. ✅ Null checks for mBlogInfo and mSubscriptionSettingsButton
2. ✅ Failed unsubscribe re-enables button (line 356-358)
3. ✅ Success path updates both state and visibility (line 346-349)
4. ✅ External feeds don't show settings (line 396: !mIsFeed)

Potential Future Improvements

1. State consistency: Consider updating mBlogInfo.isFollowing optimistically before the network call (line 318) and reverting only on failure. This would make the cached state match the UI state throughout the operation.

2. Test coverage: Add unit tests as described above to prevent regression.

3. Code organization: The toggleFollowStatus method is growing long (85 lines). Consider extracting the callback logic into a separate method for better readability (not urgent, just a maintenance consideration).

Inline Code Review

WordPress/src/main/java/org/wordpress/android/ui/reader/views/ReaderSiteHeaderView.java:320-323

// Disable subscription settings button while unsubscribing
if (!isAskingToFollow && mSubscriptionSettingsButton != null) {
    mSubscriptionSettingsButton.setEnabled(false);
}

✅ Good: Prevents user interaction during async operation. Only disables during unsubscribe (when !isAskingToFollow), which is correct since the button is already hidden during follow operations.

WordPress/src/main/java/org/wordpress/android/ui/reader/views/ReaderSiteHeaderView.java:344-349

if (succeeded) {
    // Update cached blog info and subscription settings button visibility
    if (mBlogInfo != null) {
        mBlogInfo.isFollowing = isAskingToFollow;
        updateSubscriptionSettingsButtonVisibility(mBlogInfo);
    }

✅ Excellent: This is the core fix. Updates the cached state and refreshes UI visibility. The null check is defensive and appropriate.

Minor note: Direct field mutation (mBlogInfo.isFollowing = ...) works but breaks encapsulation. Consider if ReaderBlog should have a setter method. Not a blocker since this pattern is used elsewhere in the file (line 235).

WordPress/src/main/java/org/wordpress/android/ui/reader/views/ReaderSiteHeaderView.java:355-358

// Re-enable subscription settings button if unsubscribe failed
if (!isAskingToFollow && mSubscriptionSettingsButton != null) {
    mSubscriptionSettingsButton.setEnabled(true);
}

✅ Good: Properly handles failure case by restoring button interactability. Matches the disable logic at line 321-323.

WordPress/src/main/java/org/wordpress/android/ui/reader/views/ReaderSiteHeaderView.java:398

mSubscriptionSettingsButton.setEnabled(showSettings);

✅ Good for consistency: Ensures enabled state matches visibility state. This is defensive programming and prevents edge cases where visibility and enabled state could diverge.

Verdict

Approve ✅

This is a solid bug fix that correctly addresses the reported UI inconsistencies. The implementation is clean, handles edge cases appropriately, and introduces no security or performance concerns. The main gap is test coverage, which is a broader issue for this file rather than a problem introduced by this PR.

The changes are safe to merge. I recommend adding tests in a follow-up task to prevent future regressions.

---

[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 WordPress Android by scanning the QR code below to install the corresponding build.

	App Name	WordPress Android
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr22550-f9ae925
	Build Number	1483
	Application ID	org.wordpress.android.prealpha
	Commit	f9ae925
	Installation URL	195asf2fqmoo0

Note: Google Login is not supported on these builds.

[wpmobilebot]

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

	App Name	Jetpack Android
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr22550-f9ae925
	Build Number	1483
	Application ID	com.jetpack.android.prealpha
	Commit	f9ae925
	Installation URL	57ien20d32db8

Note: Google Login is not supported on these builds.

[nbradbury]

I'm embarrassed not to have seen this problem before submitting the PR. This looks good to me and I'll approve it, and leave it up to you to decide whether to take Claude's suggestions