Skip to content

Commit 91c33cc

Browse files
committed
feat(notifications): return notification id when command execute successfully
1 parent ae29d9c commit 91c33cc

File tree

10 files changed

+53
-48
lines changed

10 files changed

+53
-48
lines changed

feature/notification/api/src/commonMain/kotlin/net/thunderbird/feature/notification/api/command/NotificationCommand.kt

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package net.thunderbird.feature.notification.api.command
22

3+
import androidx.annotation.Discouraged
34
import net.thunderbird.core.outcome.Outcome
5+
import net.thunderbird.feature.notification.api.NotificationId
46
import net.thunderbird.feature.notification.api.content.Notification
57
import net.thunderbird.feature.notification.api.receiver.NotificationNotifier
68

@@ -33,11 +35,24 @@ abstract class NotificationCommand<TNotification : Notification>(
3335
* Represents a successful command execution.
3436
*
3537
* @param TNotification The type of notification associated with the command.
38+
* @property notificationId The ID of the notification that was successfully acted upon.
3639
* @property command The command that was executed successfully.
3740
*/
3841
data class Success<out TNotification : Notification>(
42+
val notificationId: NotificationId,
3943
val command: NotificationCommand<out TNotification>,
40-
) : CommandOutcome
44+
) : CommandOutcome {
45+
companion object {
46+
@Discouraged(
47+
message = "This is a utility function to enable usage in Java code. " +
48+
"Use Success(NotificationId, NotificationCommand) instead.",
49+
)
50+
operator fun invoke(
51+
notificationId: Int,
52+
command: NotificationCommand<*>,
53+
): Success<Notification> = Success(NotificationId(notificationId), command)
54+
}
55+
}
4156

4257
/**
4358
* Represents a failed command execution.

feature/notification/api/src/commonTest/kotlin/net/thunderbird/feature/notification/api/sender/compat/NotificationSenderCompatTest.kt

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import kotlin.test.Test
1010
import kotlinx.coroutines.ExperimentalCoroutinesApi
1111
import kotlinx.coroutines.test.UnconfinedTestDispatcher
1212
import net.thunderbird.core.outcome.Outcome
13+
import net.thunderbird.feature.notification.api.NotificationId
1314
import net.thunderbird.feature.notification.api.command.NotificationCommand.Failure
1415
import net.thunderbird.feature.notification.api.command.NotificationCommand.Success
1516
import net.thunderbird.feature.notification.api.command.NotificationCommandException
@@ -25,8 +26,8 @@ class NotificationSenderCompatTest {
2526
fun `send should call listener callback whenever a result is received`() {
2627
// Arrange
2728
val expectedResults = listOf<Outcome<Success<Notification>, Failure<Notification>>>(
28-
Outcome.success(Success(FakeInAppNotificationCommand())),
29-
Outcome.success(Success(FakeSystemNotificationCommand())),
29+
Outcome.success(Success(NotificationId(1), FakeInAppNotificationCommand())),
30+
Outcome.success(Success(NotificationId(2), FakeSystemNotificationCommand())),
3031
Outcome.failure(
3132
error = Failure(
3233
command = FakeSystemNotificationCommand(),

feature/notification/api/src/jvmTest/java/net/thunderbird/feature/notification/api/sender/compat/NotificationSenderCompatJavaTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ public void send_shouldCallListenerCallback_wheneverAResultIsReceived() {
5353
? extends @NotNull Failure<? extends @NotNull Notification>
5454
>
5555
> expectedResults = List.of(
56-
Outcome.Companion.success(new Success<>(new FakeInAppNotificationCommand())),
57-
Outcome.Companion.success(new Success<>(new FakeSystemNotificationCommand())),
56+
Outcome.Companion.success(Success.Companion.invoke(1, new FakeInAppNotificationCommand())),
57+
Outcome.Companion.success(Success.Companion.invoke(2, new FakeSystemNotificationCommand())),
5858
Outcome.Companion.failure(
5959
new Failure<>(
6060
new FakeSystemNotificationCommand(),

feature/notification/impl/src/commonMain/kotlin/net/thunderbird/feature/notification/impl/command/DismissNotificationCommand.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ sealed class DismissNotificationCommand<TNotification : Notification>(
6060
"Registrar = ${notificationRegistry.registrar}"
6161
}
6262
notifier.dismiss(id)
63-
Outcome.success(Success(command = this))
63+
Outcome.success(Success(notificationId = id, command = this))
6464
}
6565

6666
else -> {

feature/notification/impl/src/commonMain/kotlin/net/thunderbird/feature/notification/impl/command/DisplayInAppNotificationCommand.kt

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,9 @@ internal class DisplayInAppNotificationCommand(
4646
)
4747

4848
canExecuteCommand() -> {
49-
notifier.show(id = notificationRegistry.register(notification), notification = notification)
50-
Outcome.success(Success(command = this))
49+
val id = notificationRegistry.register(notification)
50+
notifier.show(id = id, notification = notification)
51+
Outcome.success(Success(notificationId = id, command = this))
5152
}
5253

5354
else -> {

feature/notification/impl/src/commonMain/kotlin/net/thunderbird/feature/notification/impl/command/DisplaySystemNotificationCommand.kt

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,9 @@ internal class DisplaySystemNotificationCommand(
5252
)
5353

5454
canExecuteCommand() -> {
55-
notifier.show(
56-
id = notificationRegistry.register(notification),
57-
notification = notification,
58-
)
59-
Outcome.success(Success(command = this))
55+
val id = notificationRegistry.register(notification)
56+
notifier.show(id = id, notification = notification)
57+
Outcome.success(Success(notificationId = id, command = this))
6058
}
6159

6260
else -> {

feature/notification/impl/src/commonTest/kotlin/net/thunderbird/feature/notification/impl/command/DismissInAppNotificationCommandTest.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,8 @@ class DismissInAppNotificationCommandTest {
117117
.all {
118118
prop(Success<InAppNotification>::command)
119119
.isEqualTo(testSubject)
120+
prop(Success<InAppNotification>::notificationId)
121+
.isEqualTo(expectedId)
120122
}
121123

122124
verifySuspend(exactly(1)) { notifier.dismiss(expectedId) }

feature/notification/impl/src/commonTest/kotlin/net/thunderbird/feature/notification/impl/command/DismissSystemNotificationCommandTest.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,8 @@ class DismissSystemNotificationCommandTest {
118118
.all {
119119
prop(Success<SystemNotification>::command)
120120
.isEqualTo(testSubject)
121+
prop(Success<SystemNotification>::notificationId)
122+
.isEqualTo(expectedId)
121123
}
122124

123125
verifySuspend(exactly(1)) { notifier.dismiss(expectedId) }

feature/notification/impl/src/commonTest/kotlin/net/thunderbird/feature/notification/impl/command/DisplayInAppNotificationCommandTest.kt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,11 @@ class DisplayInAppNotificationCommandTest {
101101
severity = NotificationSeverity.Information,
102102
)
103103
val notifier = spy(FakeInAppNotificationNotifier())
104+
val notificationRegistry = FakeNotificationRegistry()
104105
val testSubject = createTestSubject(
105106
notification = notification,
106107
notifier = notifier,
108+
notificationRegistry = notificationRegistry,
107109
)
108110

109111
// Act
@@ -116,6 +118,8 @@ class DisplayInAppNotificationCommandTest {
116118
.all {
117119
prop(Success<InAppNotification>::command)
118120
.isEqualTo(testSubject)
121+
prop(Success<InAppNotification>::notificationId)
122+
.isEqualTo(notificationRegistry.getValue(notification))
119123
}
120124

121125
verifySuspend(exactly(1)) {

feature/notification/impl/src/commonTest/kotlin/net/thunderbird/feature/notification/impl/command/DisplaySystemNotificationCommandTest.kt

Lines changed: 17 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import dev.mokkery.matcher.any
1010
import dev.mokkery.spy
1111
import dev.mokkery.verify.VerifyMode.Companion.exactly
1212
import dev.mokkery.verifySuspend
13-
import kotlin.random.Random
1413
import kotlin.test.Test
1514
import kotlinx.coroutines.test.runTest
1615
import net.thunderbird.core.featureflag.FeatureFlagKey
@@ -24,10 +23,10 @@ import net.thunderbird.feature.notification.api.NotificationSeverity
2423
import net.thunderbird.feature.notification.api.command.NotificationCommand.Failure
2524
import net.thunderbird.feature.notification.api.command.NotificationCommand.Success
2625
import net.thunderbird.feature.notification.api.command.NotificationCommandException
27-
import net.thunderbird.feature.notification.api.content.Notification
2826
import net.thunderbird.feature.notification.api.content.SystemNotification
2927
import net.thunderbird.feature.notification.api.receiver.NotificationNotifier
3028
import net.thunderbird.feature.notification.testing.fake.FakeNotification
29+
import net.thunderbird.feature.notification.testing.fake.FakeNotificationRegistry
3130
import net.thunderbird.feature.notification.testing.fake.FakeSystemOnlyNotification
3231

3332
@Suppress("MaxLineLength")
@@ -134,11 +133,13 @@ class DisplaySystemNotificationCommandTest {
134133
severity = NotificationSeverity.Information,
135134
)
136135
val notifier = spy(FakeNotifier())
136+
val notificationRegistry = FakeNotificationRegistry()
137137
val testSubject = createTestSubject(
138138
notification = notification,
139139
// TODO(#9391): Verify if the app is backgrounded.
140140
isAppInBackground = { true },
141141
notifier = notifier,
142+
notificationRegistry = notificationRegistry,
142143
)
143144

144145
// Act
@@ -151,6 +152,8 @@ class DisplaySystemNotificationCommandTest {
151152
.all {
152153
prop(Success<SystemNotification>::command)
153154
.isEqualTo(testSubject)
155+
prop(Success<SystemNotification>::notificationId)
156+
.isEqualTo(notificationRegistry.getValue(notification))
154157
}
155158

156159
verifySuspend(exactly(1)) {
@@ -166,11 +169,13 @@ class DisplaySystemNotificationCommandTest {
166169
severity = NotificationSeverity.Fatal,
167170
)
168171
val notifier = spy(FakeNotifier())
172+
val notificationRegistry = FakeNotificationRegistry()
169173
val testSubject = createTestSubject(
170174
notification = notification,
171175
// TODO(#9391): Verify if the app is backgrounded.
172176
isAppInBackground = { false },
173177
notifier = notifier,
178+
notificationRegistry = notificationRegistry,
174179
)
175180

176181
// Act
@@ -183,6 +188,8 @@ class DisplaySystemNotificationCommandTest {
183188
.all {
184189
prop(Success<SystemNotification>::command)
185190
.isEqualTo(testSubject)
191+
prop(Success<SystemNotification>::notificationId)
192+
.isEqualTo(notificationRegistry.getValue(notification))
186193
}
187194

188195
verifySuspend(exactly(1)) {
@@ -198,11 +205,13 @@ class DisplaySystemNotificationCommandTest {
198205
severity = NotificationSeverity.Critical,
199206
)
200207
val notifier = spy(FakeNotifier())
208+
val notificationRegistry = FakeNotificationRegistry()
201209
val testSubject = createTestSubject(
202210
notification = notification,
203211
// TODO(#9391): Verify if the app is backgrounded.
204212
isAppInBackground = { false },
205213
notifier = notifier,
214+
notificationRegistry = notificationRegistry,
206215
)
207216

208217
// Act
@@ -215,6 +224,8 @@ class DisplaySystemNotificationCommandTest {
215224
.all {
216225
prop(Success<SystemNotification>::command)
217226
.isEqualTo(testSubject)
227+
prop(Success<SystemNotification>::notificationId)
228+
.isEqualTo(notificationRegistry.getValue(notification))
218229
}
219230

220231
verifySuspend(exactly(1)) {
@@ -230,11 +241,13 @@ class DisplaySystemNotificationCommandTest {
230241
severity = NotificationSeverity.Information,
231242
)
232243
val notifier = spy(FakeNotifier())
244+
val notificationRegistry = FakeNotificationRegistry()
233245
val testSubject = createTestSubject(
234246
notification = notification,
235247
// TODO(#9391): Verify if the app is backgrounded.
236248
isAppInBackground = { false },
237249
notifier = notifier,
250+
notificationRegistry = notificationRegistry,
238251
)
239252

240253
// Act
@@ -247,6 +260,8 @@ class DisplaySystemNotificationCommandTest {
247260
.all {
248261
prop(Success<SystemNotification>::command)
249262
.isEqualTo(testSubject)
263+
prop(Success<SystemNotification>::notificationId)
264+
.isEqualTo(notificationRegistry.getValue(notification))
250265
}
251266

252267
verifySuspend(exactly(1)) {
@@ -276,39 +291,6 @@ class DisplaySystemNotificationCommandTest {
276291
}
277292
}
278293

279-
private open class FakeNotificationRegistry : NotificationRegistry {
280-
override val registrar: Map<NotificationId, Notification>
281-
get() = error("Not yet implemented")
282-
283-
override fun get(notificationId: NotificationId): Notification? {
284-
error("Not yet implemented")
285-
}
286-
287-
override fun get(notification: Notification): NotificationId? {
288-
error("Not yet implemented")
289-
}
290-
291-
override suspend fun register(notification: Notification): NotificationId {
292-
return NotificationId(value = Random.nextInt())
293-
}
294-
295-
override fun unregister(notificationId: NotificationId) {
296-
error("Not yet implemented")
297-
}
298-
299-
override fun unregister(notification: Notification) {
300-
error("Not yet implemented")
301-
}
302-
303-
override fun contains(notification: Notification): Boolean {
304-
error("Not yet implemented")
305-
}
306-
307-
override fun contains(notificationId: NotificationId): Boolean {
308-
error("Not yet implemented")
309-
}
310-
}
311-
312294
private open class FakeNotifier : NotificationNotifier<SystemNotification> {
313295
override suspend fun show(
314296
id: NotificationId,

0 commit comments

Comments
 (0)