Skip to content

Commit

Permalink
[BugFix] Fix issue of e-mail reminder being sent event after correspo…
Browse files Browse the repository at this point in the history
…nding reminder occurrence is acknowledged.

link - personio/backend-coding-challenge#2
  • Loading branch information
Pulkit Gupta committed Jul 18, 2023
1 parent 11ef95a commit 65a5059
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,5 @@ class FindOccurrencesUseCase(
) {
fun findAll(employeeId: UUID) = occurrencesRepository.findAt(Instant.now(clock), employeeId)
}

// Notification pattern
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,7 @@ class InMemoryOccurrencesRepository(
}

override fun findAt(instant: Instant): Collection<Occurrence> {
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<Occurrence> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit 65a5059

Please sign in to comment.