-
Notifications
You must be signed in to change notification settings - Fork 137
Issue/woomob 1859 declared age range apis for app stores #15085
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Issue/woomob 1859 declared age range apis for app stores #15085
Conversation
Generated by 🚫 Danger |
Project dependencies changeslist+ New Dependencies
com.google.android.play:age-signals:0.0.2
! Upgraded Dependencies
com.google.android.gms:play-services-basement:18.9.0, (changed from 18.5.0)tree +--- com.google.firebase:firebase-messaging -> 25.0.1
| +--- com.google.firebase:firebase-common:22.0.1
| | +--- org.jetbrains.kotlinx:kotlinx-coroutines-play-services:1.9.0 -> 1.10.2
| | | \--- com.google.android.gms:play-services-tasks:16.0.1 -> 18.2.0
-| | | \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.5.0
+| | | \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.9.0
-| | \--- com.google.android.gms:play-services-basement:18.3.0 -> 18.5.0 (*)
+| | \--- com.google.android.gms:play-services-basement:18.3.0 -> 18.9.0 (*)
| +--- com.google.firebase:firebase-iid-interop:17.1.0
-| | \--- com.google.android.gms:play-services-basement:17.0.0 -> 18.5.0 (*)
+| | \--- com.google.android.gms:play-services-basement:17.0.0 -> 18.9.0 (*)
| +--- com.google.firebase:firebase-measurement-connector:19.0.0
-| | \--- com.google.android.gms:play-services-basement:17.0.0 -> 18.5.0 (*)
+| | \--- com.google.android.gms:play-services-basement:17.0.0 -> 18.9.0 (*)
| +--- com.google.android.gms:play-services-base:18.1.0 -> 18.5.0
-| | \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.5.0 (*)
+| | \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.9.0 (*)
-| +--- com.google.android.gms:play-services-basement:18.3.0 -> 18.5.0 (*)
+| +--- com.google.android.gms:play-services-basement:18.3.0 -> 18.9.0 (*)
| +--- com.google.android.gms:play-services-cloud-messaging:17.2.0
-| | \--- com.google.android.gms:play-services-basement:18.3.0 -> 18.5.0 (*)
+| | \--- com.google.android.gms:play-services-basement:18.3.0 -> 18.9.0 (*)
| \--- com.google.android.gms:play-services-stats:17.0.2
-| \--- com.google.android.gms:play-services-basement:18.0.0 -> 18.5.0 (*)
+| \--- com.google.android.gms:play-services-basement:18.0.0 -> 18.9.0 (*)
+--- com.google.firebase:firebase-config -> 23.0.1
| \--- com.google.firebase:firebase-abt:21.1.1
-| \--- com.google.android.gms:play-services-basement:18.1.0 -> 18.5.0 (*)
+| \--- com.google.android.gms:play-services-basement:18.1.0 -> 18.9.0 (*)
+--- com.google.firebase:firebase-analytics -> 23.0.0
| +--- com.google.android.gms:play-services-measurement:23.0.0
| | +--- com.google.android.gms:play-services-ads-identifier:18.0.0
-| | | \--- com.google.android.gms:play-services-basement:18.0.0 -> 18.5.0 (*)
+| | | \--- com.google.android.gms:play-services-basement:18.0.0 -> 18.9.0 (*)
-| | +--- com.google.android.gms:play-services-basement:18.5.0 (*)
+| | +--- com.google.android.gms:play-services-basement:18.5.0 -> 18.9.0 (*)
| | +--- com.google.android.gms:play-services-measurement-base:23.0.0
-| | | \--- com.google.android.gms:play-services-basement:18.5.0 (*)
+| | | \--- com.google.android.gms:play-services-basement:18.5.0 -> 18.9.0 (*)
| | +--- com.google.android.gms:play-services-measurement-impl:23.0.0
-| | | \--- com.google.android.gms:play-services-basement:18.5.0 (*)
+| | | \--- com.google.android.gms:play-services-basement:18.5.0 -> 18.9.0 (*)
| | \--- com.google.android.gms:play-services-measurement-sdk-api:23.0.0
-| | \--- com.google.android.gms:play-services-basement:18.5.0 (*)
+| | \--- com.google.android.gms:play-services-basement:18.5.0 -> 18.9.0 (*)
| +--- com.google.android.gms:play-services-measurement-api:23.0.0
-| | \--- com.google.android.gms:play-services-basement:18.5.0 (*)
+| | \--- com.google.android.gms:play-services-basement:18.5.0 -> 18.9.0 (*)
| \--- com.google.android.gms:play-services-measurement-sdk:23.0.0
-| \--- com.google.android.gms:play-services-basement:18.5.0 (*)
+| \--- com.google.android.gms:play-services-basement:18.5.0 -> 18.9.0 (*)
+--- com.google.android.gms:play-services-auth:21.4.0
| +--- com.google.android.gms:play-services-auth-api-phone:18.0.2
-| | \--- com.google.android.gms:play-services-basement:18.0.2 -> 18.5.0 (*)
+| | \--- com.google.android.gms:play-services-basement:18.0.2 -> 18.9.0 (*)
| +--- com.google.android.gms:play-services-auth-base:18.0.10
-| | \--- com.google.android.gms:play-services-basement:18.2.0 -> 18.5.0 (*)
+| | \--- com.google.android.gms:play-services-basement:18.2.0 -> 18.9.0 (*)
-| +--- com.google.android.gms:play-services-basement:18.5.0 (*)
+| +--- com.google.android.gms:play-services-basement:18.5.0 -> 18.9.0 (*)
| \--- com.google.android.gms:play-services-fido:20.0.1 -> 21.0.0
-| \--- com.google.android.gms:play-services-basement:18.3.0 -> 18.5.0 (*)
+| \--- com.google.android.gms:play-services-basement:18.3.0 -> 18.9.0 (*)
++--- com.google.android.play:age-signals:0.0.2
+| +--- com.google.android.gms:play-services-basement:18.9.0 (*)
+| +--- com.google.android.gms:play-services-tasks:18.2.0 (*)
+| \--- com.google.android.play:core-common:2.0.4
+--- project :libs:login
| \--- androidx.credentials:credentials-play-services-auth:1.5.0
| +--- com.google.android.gms:play-services-auth-blockstore:16.4.0
-| | \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.5.0 (*)
+| | \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.9.0 (*)
| \--- com.google.android.gms:play-services-identity-credentials:16.0.0-alpha02
-| \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.5.0 (*)
+| \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.9.0 (*)
+--- project :libs:cardreader
| \--- com.stripe:stripeterminal-taptopay:4.7.5
| \--- com.google.android.play:integrity:1.1.0
-| \--- com.google.android.gms:play-services-basement:18.1.0 -> 18.5.0 (*)
+| \--- com.google.android.gms:play-services-basement:18.1.0 -> 18.9.0 (*)
+--- com.google.android.play:app-update:2.1.0
-| \--- com.google.android.gms:play-services-basement:18.1.0 -> 18.5.0 (*)
+| \--- com.google.android.gms:play-services-basement:18.1.0 -> 18.9.0 (*)
+--- com.google.android.play:review:2.0.2
-| \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.5.0 (*)
+| \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.9.0 (*)
+--- com.google.android.gms:play-services-code-scanner:16.1.0
-| +--- com.google.android.gms:play-services-basement:18.1.0 -> 18.5.0 (*)
+| +--- com.google.android.gms:play-services-basement:18.1.0 -> 18.9.0 (*)
| \--- com.google.mlkit:barcode-scanning-common:17.0.0
-| +--- com.google.android.gms:play-services-basement:18.0.0 -> 18.5.0 (*)
+| +--- com.google.android.gms:play-services-basement:18.0.0 -> 18.9.0 (*)
| \--- com.google.mlkit:vision-common:17.0.0 -> 17.3.0
-| +--- com.google.android.gms:play-services-basement:18.1.0 -> 18.5.0 (*)
+| +--- com.google.android.gms:play-services-basement:18.1.0 -> 18.9.0 (*)
| \--- com.google.mlkit:common:18.6.0 -> 18.11.0
-| \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.5.0 (*)
+| \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.9.0 (*)
+--- com.google.mlkit:text-recognition:16.0.1
-| +--- com.google.android.gms:play-services-basement:18.4.0 -> 18.5.0 (*)
+| +--- com.google.android.gms:play-services-basement:18.4.0 -> 18.9.0 (*)
| +--- com.google.android.gms:play-services-mlkit-text-recognition:19.0.1
-| | +--- com.google.android.gms:play-services-basement:18.4.0 -> 18.5.0 (*)
+| | +--- com.google.android.gms:play-services-basement:18.4.0 -> 18.9.0 (*)
| | \--- com.google.android.gms:play-services-mlkit-text-recognition-common:19.1.0
-| | +--- com.google.android.gms:play-services-basement:18.4.0 -> 18.5.0 (*)
+| | +--- com.google.android.gms:play-services-basement:18.4.0 -> 18.9.0 (*)
| | \--- com.google.mlkit:vision-interfaces:16.3.0
-| | \--- com.google.android.gms:play-services-basement:18.1.0 -> 18.5.0 (*)
+| | \--- com.google.android.gms:play-services-basement:18.1.0 -> 18.9.0 (*)
| \--- com.google.mlkit:text-recognition-bundled-common:17.0.0
-| \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.5.0 (*)
+| \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.9.0 (*)
+--- com.google.android.gms:play-services-mlkit-text-recognition-japanese:16.0.1
-| \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.5.0 (*)
+| \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.9.0 (*)
+--- com.google.android.gms:play-services-mlkit-text-recognition-chinese:16.0.1
-| \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.5.0 (*)
+| \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.9.0 (*)
+--- com.google.android.gms:play-services-mlkit-text-recognition-korean:16.0.1
-| \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.5.0 (*)
+| \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.9.0 (*)
+--- com.google.android.gms:play-services-mlkit-subject-segmentation:16.0.0-beta1
-| \--- com.google.android.gms:play-services-basement:18.1.0 -> 18.5.0 (*)
+| \--- com.google.android.gms:play-services-basement:18.1.0 -> 18.9.0 (*)
+--- com.google.mlkit:barcode-scanning:17.3.0
-| +--- com.google.android.gms:play-services-basement:18.4.0 -> 18.5.0 (*)
+| +--- com.google.android.gms:play-services-basement:18.4.0 -> 18.9.0 (*)
| \--- com.google.android.gms:play-services-mlkit-barcode-scanning:18.3.1
-| \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.5.0 (*)
+| \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.9.0 (*)
+--- com.google.android.gms:play-services-wearable:19.0.0
-| \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.5.0 (*)
+| \--- com.google.android.gms:play-services-basement:18.4.0 -> 18.9.0 (*) |
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
Refactor the legacy viewmodel implementation that was previously added to LoginActivity
🤖 Test Failure AnalysisYour tests failed. Claude has analyzed the failures - check the annotation for details. |
|
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
Additionally display the restricted access dialog from LoginActivity.kt
irfano
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- It's worth adding RELEASE-NOTES for this update.
- When I change the upper age limit from 12 to 13 and open the app for the first time, the app still applies the previous behavior (age 12) and blocks access.
Similarly, when the upper age limit is changed from 14 to 12 and the app is restarted, on the first launch it behaves as if the limit were still 14 and allows access, but on the second launch it correctly treats it as 12 and blocks the app.
Screen_recording_20251217_155709.webm
All other parts look good. 👍🏻 Thanks for adding this feature.
🤖 Build Failure AnalysisThis build has failures. Claude has analyzed them - check the build annotations for details. |
|
I'm moving this to draft as I need to add bigger changes than I expected. I'll let you know once everything is ready again @irfano. And thanks for the first round 👍🏼 |
These changes ensure the restricted access dialog is shown or hide based on any changes happening to age eligibility checks
|
Hey @irfano this is ready for another round 🙏🏼
I haven't added release notes yet as I'm still not sure where these changes are going to land (23.8.1 or 23.9) |
| if (ageUpper == null) { | ||
| true // If we can't determine the age return true | ||
| } else { | ||
| ageUpper >= WOOCOMMERCE_TOS_MINIMUM_AGE_FOR_APP_USE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mentioned this on slack (p1766407567026729-slack-C03L1NF1EA3). From what I understand, if the state is SUPERVISED (approved by parent) we shouldn't check their age against WOOCOMMERCE_TOS_MINIMUM_AGE_FOR_APP_USE. For example, 12 years old user whose parents approved the app should be able to use it, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to WooCommerce TOS our app usage is expected for 13+ years old users:
Given that, the current behavior seems expected: we shouldn’t allow 12-year-old users to use the WooCommerce platform.
Test Case 3: Supervised user under 13
- Set
userStatusto SUPERVISED_APPROVAL_PENDING- Set
ageUpperto 12- Open the app
- Log in as usual
I think the last step is misleading for this case. If the user is under 13, they shouldn’t be able to “log in as usual”; they should be logged out.
@JorgeMucientes, can you confirm my understanding is correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to WooCommerce TOS our app usage is expected for 13+ years old users:
Given that, the current behavior seems expected: we shouldn’t allow 12-year-old users to use the WooCommerce platform.
Yes, that's it. Replied on Slack as well and shared the diagram that might help (will ad it to the PR desc):
I think the last step is misleading for this case. If the user is under 13, they shouldn’t be able to “log in as usual”; they should be logged out.
Well, spotted those instructions are wrong. The user should be logged out and access restricted. Updated the PR desc with the correct steps.
| import javax.inject.Inject | ||
| import javax.inject.Singleton | ||
|
|
||
| @Singleton |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ Is the singleton annotation needed here? From what I understand we inject the checker just once in the initializer. If we declare it as singleton, it'll never be garbage collected. It's a minor memory optimization, but unless we need it I'd personally replace it with @Reusable.
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/ageeligibility/AgeSignalsClient.kt
Show resolved
Hide resolved
| } | ||
|
|
||
| private fun keepTrackOfAgeEligibility() { | ||
| val dialog = AlertDialog.Builder(this) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔍 The dialog is leaking after configuration change, but considering how rare this dialog will be I think it's not worth fixing.
| } | ||
|
|
||
| @Test | ||
| fun `given user is supervised and age is under 13, when checkAge called, then user is NOT eligible`() = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is related to the behavior I mentioned above - I believe the user should be allowed to use the app, since they have been approved by their parent. Wdyt?
malinajirka
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @JorgeMucientes! Overall looks good to me. I've left some comments with suggestions + questions + one potential bug.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The incorrect “previous app state” issue is fixed now. 👍🏻 Thanks for the change. I added more comments that needs your attention. Also some reminders
- RELEASE-NOTES for the change,
- Register new tracks
| if (isUserAgeRangeEligible.value.not()) { | ||
| accountRepository.logout() | ||
| } | ||
| trackingProperties["access_restricted"] = _isUserAgeRangeEligible.value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
! is needed here.
_isUserAgeRangeEligible.value → !_isUserAgeRangeEligible.value
| <string name="age_restriction_dialog_title">Account Access Required</string> | ||
| <string name="age_restriction_dialog_message">Your Google Account settings indicate that you need a parent or guardian's permission to continue. They can grant access using their Google account to get you back online.</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we show the same message to supervised users who are under 13? In that case, the restriction isn’t related to their Google account. So I think we should either show a different message or make this one more generic.
Part of WOOMOB-1859
Description
This PR integrates Play Age Signals API in Woo Android.
The added logic works as follows:
When is a user ineligible to use WooCommerce app? Discussion p2y3YZ-anO-p2#comment-24048.
According to WooCommerce TOS our app usage is expected for 13+ years old users:
With the TOS in mind the logic to determine if a user is eligible will be based on the 4 different user statuses the API provides:
userStatus, we'll grant access by default.Here's a diagram to help better understand the different flows:
Test Steps
We currently can't test in prod, because we app needs to be downloaded from Google Play in order for the API to work. For that Google provides a
FakeAgeSignalsManager()that enables us to test the different scenarios.Apply the following patch in order to simulate the different API responses we'll get from the Play Signals API:
Use_FakeAgeSignalsManager_to_simulate_supervised_users.patch
For each of the following tests you'll need to open
AgeSignalsClient.kt.and update the mockeduserStatusTest Case 1: User 18+ (Verified)
VERIFIEDTest Case 2: Approved 13+ user (Supervised)
userStatusto SUPERVISEDageUpperto 14 (use setAgeUpper(14))Test Case 3: Supervised user under 13
userStatusto SUPERVISED_APPROVAL_PENDINGageUpperto 12Test Case 4: Supervised user approval denied
userStatusto SUPERVISED_APPROVAL_DENIEDImages/gif
Screen_recording_20251219_165448.mp4
RELEASE-NOTES.txtif necessary. Use the "[Internal]" label for non-user-facing changes.