[Hack] Add logic to BuildConfig to allow for announcement debugging.
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:
- [x] Install the apk from this PR and make sure you aren't seeing an announcement. There is currently not one in this current release,
24.7-rc-1. - [x] Now checkout this PR and build it locally.
- [x] You shouldn't see the announcement, unless you somehow upgraded the build from the previous version to
24.7. I created a test announcement for you for release24.7. - [x] Add
alwaysShowAnnouncement=trueto yourlocal.propertiesfile and rebuild. You should be seeing the announcement pop up on app load everytime.
Extra credit:
- [ ] Rebuild the app in release mode and leave the
alwaysShowAnnouncement=true. You should not be getting the announcement. This covers us in the event it slips into production. Should be impossible consideringlocal.propertieswould need also have thealwaysShowAnnouncementin CI when producing release builds.
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.txtif necessary.
Quality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code
📲 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 |
📲 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 |
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).
Additional details and impacted files
@@ 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.
👋 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.
@notandyvee, I am the reviewer but the PR is still draft. Is this ready for review?
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.
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.
I added alwaysShowAnnouncement=true to local.properties and built jetpackWasabiDebug but couldn't see the announcement. What am I missing? 🤔
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 ?
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.
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.
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.
It appeared to me on trunk build once. Is that the upgrade to 24.7 scenario showing up?
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.
@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.
@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, 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.
@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?
@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.
Quality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code