Part-1: Compiler Warnings as Errors - WordPress Module - FragmentManager by 07jasjeet · Pull Request #19471 · wordpress-mobile/WordPress-Android

07jasjeet · 2023-10-27T20:12:35Z

Partially #17253 (Sub-ticket of #17246)

Replaced deprecated fragmentManager API with parentFragmentManager or childFragmentManager which are direct recommended replacements for above mentioned deprecated API.
This replacement required migration of AppRatingDialog from android.app.DialogFragment to androidx.fragment.app.DialogFragment and hence it is done so.

Regression Notes

Potential unintended areas of impact
Mostly none because the newer API's are a direct replacement.
What I did to test those areas of impact (or what existing automated tests I relied on)
Manual Testing
What automated tests I added (or what prevented me from doing so)
None

PR submission checklist:

I have completed the Regression Notes.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

UI Changes testing checklist:

Portrait and landscape orientations.
Light and dark modes.
Fonts: Larger, smaller and bold text.
High contrast.
Talkback.
Languages with large words or with letters/accents not frequently used in English.
Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
Large and small screen sizes. (Tablet and smaller phones)
Multi-tasking: Split screen and Pop-up view. (Android 10 or higher)

1) Replaced parentFragmentManager with ChildFragmentManager 2) Removed redundant java default parameter function with kotlin default parameters.

07jasjeet · 2023-10-28T06:51:56Z

@ParaskP7 @antonis

1) Reverted default constructor removal. 2) Pushing un-pushed changes, i.e., migrated deprecated getFragmentManager API usage to getSupportFragmentManager

antonis · 2023-10-31T08:36:42Z

Thank you for stepping forward to help with this issue @07jasjeet 🙇
My plan is to fully review and test this PR by the end of the week.

I've started by triggering the CI tests and noticed the following minor checkstyle issues.

> Task :WordPress:checkstyle
[ant:checkstyle] [ERROR] /Users/antonis/git/WordPress-Android/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java:18: 'androidx.activity.OnBackPressedCallback' should be separated from previous imports. [ImportOrder]
[ant:checkstyle] [ERROR] /Users/antonis/git/WordPress-Android/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java:26: 'com.google.android.gms.common.ConnectionResult' should be separated from previous imports. [ImportOrder]
[ant:checkstyle] [ERROR] /Users/antonis/git/WordPress-Android/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java:29: 'org.greenrobot.eventbus.EventBus' should be separated from previous imports. [ImportOrder]

You can find some information on how to run those checks locally here

Checkstyle complaint resolved.

antonis

Hey @07jasjeet 👋
Thanks again for picking this up. I've started testing the changes today and noticed a crash with the PrivateAtCookieRefreshProgressDialog when triggered from the ReaderPostDetailFragment. You can find some details on my testing below. In some cases I tweaked the app in debug mode for my tests since reproducing the exact conditions when the specific screens appear might be difficult.

WPMainActivity
StatsMinifiedWidgetConfigureFragment (Jetpack app flavour)
Tested by adding a new mini widget (the first one) on the home screen
StatsWidgetConfigureFragment (Jetpack app flavour)
Tested by adding a new widget on the home screen
AppRatingDialog
Triggered in WPMainActivity
Tested by tweaking the conditions here and here.
PrivateAtCookieRefreshProgressDialog:
Triggered in:
- [✓]WPWebViewActivity: Tested by changing the condition here and using the Google Domains>Transfer your domains (Jetpack app flavour) option from the home screen
- [✓]EditPostActivity: Tested by tweaking the condition here and creating a new post.
- [X]ReaderPostDetailFragment (Jetpack app flavour): I tried forcibly showing the dialog by tweaking the if statement here and opening a reader post details and got the following exception.

java.lang.IllegalStateException: Fragment PrivateAtCookieRefreshProgressDialog{dbd370d} (d24e5cc0-9ff2-4557-b509-28cd1c11622d tag=private_at_cookie_progress_dialog) declared target fragment ReaderPostDetailFragment{ebcab11} (4ff3faff-6c67-4862-bb43-9ca97331fb16 id=0x7f0a0ba9) that does not belong to this FragmentManager!
	at androidx.fragment.app.FragmentStateManager.attach(FragmentStateManager.java:441)
	at androidx.fragment.app.FragmentStateManager.moveToExpectedState(FragmentStateManager.java:254)
	at androidx.fragment.app.FragmentManager.executeOpsTogether(FragmentManager.java:1901)
	at androidx.fragment.app.FragmentManager.removeRedundantOperationsAndExecute(FragmentManager.java:1825)
	at androidx.fragment.app.FragmentManager.execPendingActions(FragmentManager.java:1762)
	at androidx.fragment.app.FragmentManager$5.run(FragmentManager.java:547)
	at android.os.Handler.handleCallback(Handler.java:958)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loopOnce(Looper.java:205)
	at android.os.Looper.loop(Looper.java:294)
	at android.app.ActivityThread.main(ActivityThread.java:8177)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:552)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:971)

Also feel free to add test instructions at the description of the PR 🙇

07jasjeet · 2023-11-01T17:08:51Z

[X]ReaderPostDetailFragment (Jetpack app flavour): I tried forcibly showing the dialog by tweaking the if statement here and opening a reader post details and got the following exception.

@antonis Fixed!

WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderPostDetailFragment.kt

antonis

Thank you for the changes @07jasjeet 🙇
I've retested the PrivateAtCookieRefreshProgressDialog and everything worked for me:

WPWebViewActivity
EditPostActivity
ReaderPostDetailFragment

The code changes also look good 🎉

Apart from the minor detekt issue I reported all look good :)

antonis

Awesome work @07jasjeet 🏅
Everything looks goos on my side 🎉

07jasjeet added 5 commits October 28, 2023 01:15

Update ReaderPostDetailFragment.kt

f3543a3

Update StatsWidgetConfigureFragment.kt

bfc5ab5

Update StatsMinifiedWidgetConfigureFragment.kt

9f1dcb2

Update AppRatingDialog.kt

25e1989

Cleanup

4c18780

07jasjeet changed the title ~~Compiler~~ Compiler Warnings as Errors - WordPress Module - FragmentManager Oct 27, 2023

Changes

c65d83d

1) Replaced parentFragmentManager with ChildFragmentManager 2) Removed redundant java default parameter function with kotlin default parameters.

07jasjeet added 3 commits October 28, 2023 12:27

Update ReaderPostDetailFragment.kt

c55e145

Changes

86c3ddf

1) Reverted default constructor removal. 2) Pushing un-pushed changes, i.e., migrated deprecated getFragmentManager API usage to getSupportFragmentManager

Migrated parentFragmentManager to childFragmentManager

9178ba3

antonis self-requested a review October 31, 2023 08:12

antonis added Tooling Tech Debt [Type] Enhancement [Type] Task and removed [Type] Enhancement labels Oct 31, 2023

ParaskP7 mentioned this pull request Oct 31, 2023

Compiler Warnings as Errors - WordPress Module - FragmentManager #17253

Closed

07jasjeet changed the title ~~Compiler Warnings as Errors - WordPress Module - FragmentManager~~ Compiler Warnings as Errors - WordPress Module - FragmentManager Part-1 Oct 31, 2023

07jasjeet changed the title ~~Compiler Warnings as Errors - WordPress Module - FragmentManager Part-1~~ Part-1: Compiler Warnings as Errors - WordPress Module - FragmentManager Oct 31, 2023

antonis added this to the 23.7 milestone Oct 31, 2023

Undo cleanup

093647d

Checkstyle complaint resolved.

07jasjeet mentioned this pull request Oct 31, 2023

Part-2: Compiler Warnings as Errors - WordPress Module - FragmentManager #19490

Merged

12 tasks

antonis suggested changes Nov 1, 2023

View reviewed changes

Fixed ReaderPostDetailFragment.kt crash

d8554b3

antonis self-requested a review November 2, 2023 09:33

antonis reviewed Nov 2, 2023

View reviewed changes

WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderPostDetailFragment.kt Outdated Show resolved Hide resolved

antonis reviewed Nov 2, 2023

View reviewed changes

Fix imports in ReaderPostDetailFragment.kt

f4564b5

antonis self-requested a review November 2, 2023 10:48

antonis approved these changes Nov 2, 2023

View reviewed changes

antonis merged commit 8938667 into wordpress-mobile:trunk Nov 2, 2023

07jasjeet deleted the 17246-compiler-warnings branch November 7, 2023 12:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Part-1: Compiler Warnings as Errors - WordPress Module - FragmentManager #19471

Part-1: Compiler Warnings as Errors - WordPress Module - FragmentManager #19471
antonis merged 12 commits intowordpress-mobile:trunkfrom
07jasjeet:17246-compiler-warnings

07jasjeet commented Oct 27, 2023 •

edited by antonis

Loading

Uh oh!

07jasjeet commented Oct 28, 2023

Uh oh!

antonis commented Oct 31, 2023

Uh oh!

antonis left a comment

Uh oh!

07jasjeet commented Nov 1, 2023

Uh oh!

Uh oh!

antonis left a comment

Uh oh!

antonis left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

07jasjeet commented Oct 27, 2023 • edited by antonis Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Regression Notes

Uh oh!

07jasjeet commented Oct 28, 2023

Uh oh!

antonis commented Oct 31, 2023

Uh oh!

antonis left a comment

Choose a reason for hiding this comment

Uh oh!

07jasjeet commented Nov 1, 2023

Uh oh!

Uh oh!

antonis left a comment

Choose a reason for hiding this comment

Uh oh!

antonis left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

07jasjeet commented Oct 27, 2023 •

edited by antonis

Loading