-
Notifications
You must be signed in to change notification settings - Fork 137
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
Issue/13213 disable notifications api request #13220
base: trunk
Are you sure you want to change the base?
Issue/13213 disable notifications api request #13220
Conversation
Generated by 🚫 Danger |
Project dependencies changesThe following changes in project dependencies were detected (configuration list
tree-+--- org.wordpress:fluxc:trunk-b329e031481c8653a57c0a41c4dedfc0ba6a510e
-| +--- org.jetbrains.kotlin:kotlin-parcelize-runtime:1.9.25 -> 2.0.21
-| | +--- org.jetbrains.kotlin:kotlin-stdlib:2.0.21 (*)
-| | \--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:2.0.21
-| | \--- org.jetbrains.kotlin:kotlin-stdlib:2.0.21 (*)
-| +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.6
-| | \--- androidx.annotation:annotation:1.2.0 -> 1.8.1 (*)
-| +--- androidx.security:security-crypto:1.0.0 -> 1.1.0-alpha03
-| | +--- androidx.annotation:annotation:1.1.0 -> 1.8.1 (*)
-| | +--- com.google.crypto.tink:tink-android:1.5.0
-| | \--- androidx.collection:collection:1.1.0 -> 1.4.0 (*)
-| +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0
-| | +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.12.0 (*)
-| | \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.4.10 -> 1.9.10 (*)
-| +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
-| +--- org.apache.commons:commons-text:1.10.0 (*)
-| +--- androidx.room:room-runtime:2.6.1 (*)
-| +--- androidx.room:room-ktx:2.6.1
-| | +--- androidx.room:room-common:2.6.1 (*)
-| | +--- androidx.room:room-runtime:2.6.1 (*)
-| | +--- org.jetbrains.kotlin:kotlin-stdlib:1.8.22 -> 2.0.21 (*)
-| | +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.7.1 -> 1.8.1 (*)
-| | +--- androidx.room:room-common:2.6.1 (c)
-| | \--- androidx.room:room-runtime:2.6.1 (c)
-| +--- com.google.dagger:dagger:2.51.1
-| | \--- javax.inject:javax.inject:1
-| +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.8.1 (*)
-| +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.8.1 (*)
-| +--- org.wordpress:wellsql:2.0.0
-| | +--- androidx.annotation:annotation:1.2.0 -> 1.8.1 (*)
-| | \--- org.wordpress.wellsql:wellsql-annotations:2.0.0
-| +--- org.wordpress.fluxc:fluxc-annotations:trunk-b329e031481c8653a57c0a41c4dedfc0ba6a510e
-| +--- org.greenrobot:eventbus:3.3.1 (*)
-| +--- org.greenrobot:eventbus-java:3.3.1
-| +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.12.0 (*)
-| +--- com.android.volley:volley:1.1.1 -> 1.2.0
-| +--- androidx.paging:paging-runtime:2.1.2
-| | +--- androidx.paging:paging-common:2.1.2
-| | | +--- androidx.annotation:annotation:1.0.0 -> 1.8.1 (*)
-| | | \--- androidx.arch.core:core-common:2.0.0 -> 2.2.0 (*)
-| | +--- androidx.arch.core:core-runtime:2.0.0 -> 2.2.0 (*)
-| | +--- androidx.lifecycle:lifecycle-runtime:2.0.0 -> 2.8.7 (*)
-| | +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.8.7 (*)
-| | \--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
-| +--- com.goterl:lazysodium-android:5.0.2
-| +--- net.java.dev.jna:jna:5.5.0
-| \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.25 -> 2.0.21 (*)
++--- org.wordpress:fluxc:3122-c8c9f508066c0c8165420dee180b7d10cafbfbb6
+| +--- org.jetbrains.kotlin:kotlin-parcelize-runtime:1.9.25 -> 2.0.21
+| | +--- org.jetbrains.kotlin:kotlin-stdlib:2.0.21 (*)
+| | \--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:2.0.21
+| | \--- org.jetbrains.kotlin:kotlin-stdlib:2.0.21 (*)
+| +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.6
+| | \--- androidx.annotation:annotation:1.2.0 -> 1.8.1 (*)
+| +--- androidx.security:security-crypto:1.0.0 -> 1.1.0-alpha03
+| | +--- androidx.annotation:annotation:1.1.0 -> 1.8.1 (*)
+| | +--- com.google.crypto.tink:tink-android:1.5.0
+| | \--- androidx.collection:collection:1.1.0 -> 1.4.0 (*)
+| +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0
+| | +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.12.0 (*)
+| | \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.4.10 -> 1.9.10 (*)
+| +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
+| +--- org.apache.commons:commons-text:1.10.0 (*)
+| +--- androidx.room:room-runtime:2.6.1 (*)
+| +--- androidx.room:room-ktx:2.6.1
+| | +--- androidx.room:room-common:2.6.1 (*)
+| | +--- androidx.room:room-runtime:2.6.1 (*)
+| | +--- org.jetbrains.kotlin:kotlin-stdlib:1.8.22 -> 2.0.21 (*)
+| | +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.7.1 -> 1.8.1 (*)
+| | +--- androidx.room:room-common:2.6.1 (c)
+| | \--- androidx.room:room-runtime:2.6.1 (c)
+| +--- com.google.dagger:dagger:2.51.1
+| | \--- javax.inject:javax.inject:1
+| +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.8.1 (*)
+| +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.8.1 (*)
+| +--- org.wordpress:wellsql:2.0.0
+| | +--- androidx.annotation:annotation:1.2.0 -> 1.8.1 (*)
+| | \--- org.wordpress.wellsql:wellsql-annotations:2.0.0
+| +--- org.wordpress.fluxc:fluxc-annotations:3122-c8c9f508066c0c8165420dee180b7d10cafbfbb6
+| +--- org.greenrobot:eventbus:3.3.1 (*)
+| +--- org.greenrobot:eventbus-java:3.3.1
+| +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.12.0 (*)
+| +--- com.android.volley:volley:1.1.1 -> 1.2.0
+| +--- androidx.paging:paging-runtime:2.1.2
+| | +--- androidx.paging:paging-common:2.1.2
+| | | +--- androidx.annotation:annotation:1.0.0 -> 1.8.1 (*)
+| | | \--- androidx.arch.core:core-common:2.0.0 -> 2.2.0 (*)
+| | +--- androidx.arch.core:core-runtime:2.0.0 -> 2.2.0 (*)
+| | +--- androidx.lifecycle:lifecycle-runtime:2.0.0 -> 2.8.7 (*)
+| | +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.8.7 (*)
+| | \--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
+| +--- com.goterl:lazysodium-android:5.0.2
+| +--- net.java.dev.jna:jna:5.5.0
+| \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.25 -> 2.0.21 (*)
-\--- org.wordpress.fluxc.plugins:woocommerce:trunk-b329e031481c8653a57c0a41c4dedfc0ba6a510e
- +--- org.wordpress:fluxc:trunk-b329e031481c8653a57c0a41c4dedfc0ba6a510e (*)
- +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
- +--- com.google.dagger:dagger:2.51.1 (*)
- +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.8.1 (*)
- +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.8.1 (*)
- +--- androidx.room:room-runtime:2.6.1 (*)
- +--- org.wordpress:wellsql:2.0.0 (*)
- +--- org.wordpress.fluxc:fluxc-annotations:trunk-b329e031481c8653a57c0a41c4dedfc0ba6a510e
- +--- androidx.room:room-ktx:2.6.1 (*)
- \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.25 -> 2.0.21 (*)
+\--- org.wordpress.fluxc.plugins:woocommerce:3122-c8c9f508066c0c8165420dee180b7d10cafbfbb6
+ +--- org.wordpress:fluxc:3122-c8c9f508066c0c8165420dee180b7d10cafbfbb6 (*)
+ +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
+ +--- com.google.dagger:dagger:2.51.1 (*)
+ +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.8.1 (*)
+ +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.8.1 (*)
+ +--- androidx.room:room-runtime:2.6.1 (*)
+ +--- org.wordpress:wellsql:2.0.0 (*)
+ +--- org.wordpress.fluxc:fluxc-annotations:3122-c8c9f508066c0c8165420dee180b7d10cafbfbb6
+ +--- androidx.room:room-ktx:2.6.1 (*)
+ \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.25 -> 2.0.21 (*) |
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
Testing notes: Both success and failure test cases work in my test. With success, I can confirm that testing with two devices (one Simulator and one real), the one with disabled notification did not receive it while the other received it. With the failure I tested with API faker and it worked as well. Additionally, I like the loading animation being put on the button (which conveniently hides the button too), and the retry dialog is simple but effective 👍🏼 |
testBlocking { | ||
val event = viewModel.event.runAndCaptureValues { | ||
viewModel.onSaveTapped() | ||
}.last() | ||
|
||
assertThat(event).isEqualTo(ExitWithResult(data = true)) | ||
} | ||
|
||
@Test | ||
fun `given updating notification settings fails, when tapping save, then exit with result`() = |
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.
nit, should be then show error dialog
.
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.
Good catch! Fixed!
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.
code looks good, pre-approving 👍🏼 👍🏼
left a small comment about unit test name but all good otherwise.
Closes: #13213
Description
Integrates new FluxC changes to disable:
When a site is disabled from the list of visible sites. Notifications will only be disabled for the current device used to hide the sites.
Testing information
Basic steps
Notification settings updated successfully case
Notification settings failed to update case
errorHandling.mp4
The tests that have been performed
Images/gif
(Demo adding delay to properly show the loading spinner)
LoadingState.mp4
RELEASE-NOTES.txt
if necessary. Use the "[Internal]" label for non-user-facing changes.Reviewer (or Author, in the case of optional code reviews):
Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement: