[Hack] Add logic to BuildConfig to allow for announcement debugging.

notandyvee commented 2 months ago

The current announcement logic is a bit of a pain to test. Normally I'd add a feature flag. But I thought instead let's add something in build.gradle to use local.properties so it's a bit easier to test.

Now you can simply add alwaysShowAnnouncement=true to your local.properties, build the app in debug, and the announcements from your current release should be shown every time you load the app.

Thoughts?

To Test:

[ ] Go to the Me tab
[ ] Debug Settings
[ ] Press the overflow menu and click All shard preference flag.
[ ] You should see prefs_always_show_announcement. Make sure it's checked.
[ ] When you exit the debug settings screen you will begin to see the announcement pop up often.

Note: I was going to add documentation, but instead decided against it until this PR is reviewed.

Regression Notes

Potential unintended areas of impact

n/a
What I did to test those areas of impact (or what existing automated tests I relied on)

Manual test.
What automated tests I added (or what prevented me from doing so)

n/a

PR Submission Checklist:

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

sonarcloud[bot] commented 2 months ago

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

wpmobilebot commented 2 months ago

📲 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	pr20486-5518e1f
	Commit	5518e1fd9fbae7fabd6a94a55a6dd610d1301189
	Direct Download	`wordpress-prototype-build-pr20486-5518e1f.apk`

Note: Google Login is not supported on these builds.

wpmobilebot commented 2 months ago

📲 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	pr20486-5518e1f
	Commit	5518e1fd9fbae7fabd6a94a55a6dd610d1301189
	Direct Download	`jetpack-prototype-build-pr20486-5518e1f.apk`

Note: Google Login is not supported on these builds.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 45.45455% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 40.66%. Comparing base (6c352ca) to head (5518e1f).

Files	Patch %	Lines
.../android/viewmodel/main/WPMainActivityViewModel.kt	57.14%	0 Missing and 3 partials :warning:
.../org/wordpress/android/ui/prefs/AppPrefsWrapper.kt	33.33%	2 Missing :warning:
.../java/org/wordpress/android/ui/prefs/AppPrefs.java	0.00%	1 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## trunk #20486 +/- ## ======================================= Coverage 40.66% 40.66% ======================================= Files 1490 1490 Lines 68621 68629 +8 Branches 11338 11340 +2 ======================================= + Hits 27907 27911 +4 - Misses 38195 38198 +3 - Partials 2519 2520 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

guarani commented 1 month ago

👋 My setup wasn't working so I couldn't test. See https://a8c.slack.com/archives/C441E3YTS/p1711413127791989?thread_ts=1711405412.749429&cid=C441E3YTS for details.

Given this, I think it would be best if @irfano could review 🙏 , so I removed myself as a reviewer.

irfano commented 1 month ago

@notandyvee, I am the reviewer but the PR is still draft. Is this ready for review?

notandyvee commented 1 month ago

Apologies @irfano I just haven't had time. Let me carve out time today to make the documentation and then convert it to a final PR. Since Paul was having trouble it became clear it needs some documentation.

notandyvee commented 1 month ago

It's ready @irfano . I held off on writing some docs until I hear what you think. Please note this PR does not need to be merged. If you feel strongly against adding this, we can close and consider a different way of accomplishing the same thing. No pressure. Adding @aditi-bhatia to the reviewers to get some more opinions.

irfano commented 1 month ago

I added alwaysShowAnnouncement=true to local.properties and built jetpackWasabiDebug but couldn't see the announcement. What am I missing? 🤔

notandyvee commented 1 month ago

I added alwaysShowAnnouncement=true to local.properties and built jetpackWasabiDebug but couldn't see the announcement. What am I missing? 🤔

I don't use wasabi. This could be another area of concern. Could you try jalapeno first to see whatsup @irfano ?

irfano commented 1 month ago

Our purpose of this is to see how the announcement will look on the screen, right? I find putting a config in the Debug settings screen preferable because testers who are not Android developers can also utilize this setting. WDYT?

I noticed that iOS has some buttons for similar features on its Debug settings screen. We can do the same.

notandyvee commented 1 month ago

I noticed that iOS has some buttons for similar features on its Debug settings screen. We can do the same.

I'll need some time to look into how iOS is doing it. Android's is a bit clunky, which is why I decided against it. But sure.

irfano commented 1 month ago

We have a "FEATURES IN DEVELOPMENT" tab in the "Debug settings". Perhaps we can add a fourth tab to this screen, "LOCAL CONFIGS", and use the same design as "FEATURES IN DEVELOPMENT.

I believe being able to find everything related to testing configs within "Debug settings" would be a better option.

ravishanker commented 3 weeks ago

It appeared to me on trunk build once. Is that the upgrade to 24.7 scenario showing up?

ravishanker commented 3 weeks ago

We have a "FEATURES IN DEVELOPMENT" tab in the "Debug settings". Perhaps we can add a fourth tab to this screen, "LOCAL CONFIGS", and use the same design as "FEATURES IN DEVELOPMENT.

I believe being able to find everything related to testing configs within "Debug settings" would be a better option.

You could just add it to Features in development tab, and stays there forever.

notandyvee commented 1 week ago

@irfano sorry I didn't get to this sooner. Mind taking a look when you get a chance and give me your thoughts. I did what was mentioned. Add a new tab on the debug settings screen. I decided to create a new UiItem, SharedPrefsField. This item currently only represents a boolean, but can be a string, long, etc. Hence why I am using a type. I tried to keep it simple since it's only for debugging.

irfano commented 1 week ago

@notandyvee, while reviewing the change, I noticed a screen that already contains your new pref, pref_always_show_announcement. This screen was introduced 4 months ago (#19880). Do you think we can just add a new pref and use this existing screen?

notandyvee commented 1 week ago

@notandyvee, while reviewing the change, I noticed a screen that already contains your new pref, pref_always_show_announcement. This screen was introduced 4 months ago (https://github.com/wordpress-mobile/WordPress-Android/pull/19880). Do you think we can just add a new pref and use this existing screen?

Sigh 🤦🏻‍♂️ . I do think we should re-use that screen. Just sucks we didn't catch that sooner. Gonna be the 3rd iteration. I'll look into it on monday.

notandyvee commented 5 days ago

@justtwago I made some edits to DebugSharedPreferenceFlagsViewModel that require your approval. I needed a debug shared pref value for debugging. Since the shared pref needs to be written first before it's seen on that screen, I thought it made more sense to add any flags we want to test in a list of enums. This way any test ones can be toggled.

The other big reason I felt this change would be helpful is to open it up for strings and other primitive types. Maybe test urls or other text we can change for debug purposed. I didn't make that change as it's not required yet, but it's why I added KClass in the DebugPrefs enum. Thoughts?

notandyvee commented 4 days ago

@irfano I'll fix the failing test.

What does "Class used to track" mean?

I started to add class doc and stopped mid way 😅 . Updated now.

Now, I can only see pres_always_show_announcement

This is on purpose. It's why I pinged @justtwago . Currently the preference flag needs to exist in order for it to show up. I just updated it to be more explicit. If you have no issues feel free to approve, but do not merge. We can wait for Arty to put his opinion.

sonarcloud[bot] commented 4 days ago

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

wordpress-mobile / WordPress-Android