From 65a5059f82164b4f954eef74ef53e1c8b912f5e2 Mon Sep 17 00:00:00 2001 From: Pulkit Gupta Date: Tue, 18 Jul 2023 21:43:28 +0530 Subject: [PATCH] [BugFix] Fix issue of e-mail reminder being sent event after corresponding reminder occurrence is acknowledged. link - https://github.com/personio/backend-coding-challenge/issues/2 --- .../http/v1/requests/CreateReminderRequest.kt | 2 ++ .../email/SendOccurrencesByEmailUseCase.kt | 1 + .../find/FindOccurrencesUseCase.kt | 2 ++ .../InMemoryOccurrencesRepository.kt | 5 +--- .../OccurrencesRepositoryContractTest.kt | 23 +++++++++++++++++++ .../SendOccurrencesByEmailUseCaseTest.kt | 21 +++++++++++++++++ 6 files changed, 50 insertions(+), 4 deletions(-) diff --git a/src/main/kotlin/com/personio/reminders/api/http/v1/requests/CreateReminderRequest.kt b/src/main/kotlin/com/personio/reminders/api/http/v1/requests/CreateReminderRequest.kt index 4d791b9..a603b3a 100644 --- a/src/main/kotlin/com/personio/reminders/api/http/v1/requests/CreateReminderRequest.kt +++ b/src/main/kotlin/com/personio/reminders/api/http/v1/requests/CreateReminderRequest.kt @@ -2,8 +2,10 @@ package com.personio.reminders.api.http.v1.requests import com.fasterxml.jackson.annotation.JsonProperty import java.util.UUID +import org.jetbrains.annotations.NotNull data class CreateReminderRequest( + @NotNull val text: String, @JsonProperty("employee_id") val employeeId: UUID, diff --git a/src/main/kotlin/com/personio/reminders/usecases/occurrences/email/SendOccurrencesByEmailUseCase.kt b/src/main/kotlin/com/personio/reminders/usecases/occurrences/email/SendOccurrencesByEmailUseCase.kt index bd58350..a497f1f 100644 --- a/src/main/kotlin/com/personio/reminders/usecases/occurrences/email/SendOccurrencesByEmailUseCase.kt +++ b/src/main/kotlin/com/personio/reminders/usecases/occurrences/email/SendOccurrencesByEmailUseCase.kt @@ -29,6 +29,7 @@ class SendOccurrencesByEmailUseCase( @Scheduled(cron = "0 */5 * * * *") fun sendReminders() = occurrences.findAt(Instant.now(clock)) .filter { !it.isNotificationSent } + .filter { !it.isAcknowledged } .forEach { occurrence -> val message = Message(occurrence.reminder.text, occurrence.reminder.employeeId) mailer.send(message) diff --git a/src/main/kotlin/com/personio/reminders/usecases/occurrences/find/FindOccurrencesUseCase.kt b/src/main/kotlin/com/personio/reminders/usecases/occurrences/find/FindOccurrencesUseCase.kt index e194f22..3484a98 100644 --- a/src/main/kotlin/com/personio/reminders/usecases/occurrences/find/FindOccurrencesUseCase.kt +++ b/src/main/kotlin/com/personio/reminders/usecases/occurrences/find/FindOccurrencesUseCase.kt @@ -20,3 +20,5 @@ class FindOccurrencesUseCase( ) { fun findAll(employeeId: UUID) = occurrencesRepository.findAt(Instant.now(clock), employeeId) } + +// Notification pattern diff --git a/src/test/kotlin/com/personio/reminders/infra/postgres/occurrences/InMemoryOccurrencesRepository.kt b/src/test/kotlin/com/personio/reminders/infra/postgres/occurrences/InMemoryOccurrencesRepository.kt index 62f732a..4152807 100644 --- a/src/test/kotlin/com/personio/reminders/infra/postgres/occurrences/InMemoryOccurrencesRepository.kt +++ b/src/test/kotlin/com/personio/reminders/infra/postgres/occurrences/InMemoryOccurrencesRepository.kt @@ -31,10 +31,7 @@ class InMemoryOccurrencesRepository( } override fun findAt(instant: Instant): Collection { - return occurrences.filter { - !it.isAcknowledged && - Instant.parse(it.date).isBefore(instant) - } + return occurrences.filter { Instant.parse(it.date).isBefore(instant) } } override fun findAt(instant: Instant, employeeId: UUID): Collection { diff --git a/src/test/kotlin/com/personio/reminders/infra/postgres/occurrences/OccurrencesRepositoryContractTest.kt b/src/test/kotlin/com/personio/reminders/infra/postgres/occurrences/OccurrencesRepositoryContractTest.kt index 308d022..2cdd57e 100644 --- a/src/test/kotlin/com/personio/reminders/infra/postgres/occurrences/OccurrencesRepositoryContractTest.kt +++ b/src/test/kotlin/com/personio/reminders/infra/postgres/occurrences/OccurrencesRepositoryContractTest.kt @@ -65,6 +65,29 @@ interface OccurrencesRepositoryContractTest { assertEquals(reminderForEmployee1.id, foundOccurrences.single().reminder.id) } + @Test + fun `find at should return all occurrences`() { + val reminderForEmployee1 = MotherObject.reminders().new() + val reminderForEmployee2 = MotherObject.reminders().new() + val occurrenceForEmployee1 = MotherObject.occurrences().newFrom(reminderForEmployee1) + val occurrenceForEmployee2 = MotherObject.occurrences().newFrom(reminderForEmployee2) + val repo = subjectWithData( + listOf( + reminderForEmployee1, + reminderForEmployee2 + ), + listOf( + occurrenceForEmployee1, + occurrenceForEmployee2 + ), + MotherObject.clock + ) + + val foundOccurrences = repo.findAt(Instant.now(MotherObject.clock)) + + assertEquals(0, foundOccurrences.size) + } + @Test fun `should detect reminders to recur`() { val date = Instant.now(MotherObject.clock) diff --git a/src/test/kotlin/com/personio/reminders/usecases/occurrences/email/SendOccurrencesByEmailUseCaseTest.kt b/src/test/kotlin/com/personio/reminders/usecases/occurrences/email/SendOccurrencesByEmailUseCaseTest.kt index 6185b8f..b1c810a 100644 --- a/src/test/kotlin/com/personio/reminders/usecases/occurrences/email/SendOccurrencesByEmailUseCaseTest.kt +++ b/src/test/kotlin/com/personio/reminders/usecases/occurrences/email/SendOccurrencesByEmailUseCaseTest.kt @@ -93,6 +93,27 @@ class SendOccurrencesByEmailUseCaseTest { verify(mailer, times(1)).send(any()) } + @Test + fun `it should not send mail for already acknowledge occurrences`() { + val reminderTwo = MotherObject.reminders().new(text = "Remind me of creating tests") + val occurrenceTwo = MotherObject.occurrences().newFrom(reminderTwo, isAcknowledged = true) + val occurrences = InMemoryOccurrencesRepository( + mutableListOf(reminderTwo), + mutableListOf(occurrenceTwo), + MotherObject.clock + ) + val mailer: MailerService = mock() + val subject = SendOccurrencesByEmailUseCase( + occurrences, + Clock.offset(MotherObject.clock, Duration.ofSeconds(1L)), + mailer + ) + + subject.sendReminders() + + verify(mailer, times(0)).send(any()) + } + @Test fun `it should mark occurrence as already notified`() { val reminderOne = MotherObject.reminders().new()