Skip to content

Commit 4b91418

Browse files
vitormattosvitormattos
authored
Merge pull request #5504 from LibreSign/chore/cover-with-more-scenarios
chore: cover with more scenarios
2 parents 6c75dac + 6ef9c23 commit 4b91418

2 files changed

Lines changed: 136 additions & 12 deletions

File tree

‎lib/Service/ReminderService.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,10 @@ protected function willNotify(array $summarized, \DateTime $now, int $daysBefore
184184
return false;
185185
}
186186

187+
if ($daysBetween === 0) {
188+
return false;
189+
}
190+
187191
if ($summarized['total'] === 1) {
188192
return $this->shouldNotifyAfterSomeDays($summarized['first'], $now, $daysBefore);
189193
}
@@ -197,7 +201,7 @@ private function isMaxReached(int $total, int $max): bool {
197201

198202
private function shouldNotifyAfterSomeDays(?\DateTime $date, \DateTime $now, int $maxDays): bool {
199203
$daysAfter = $date?->diff($now)?->days ?? 0;
200-
return $daysAfter > $maxDays;
204+
return $maxDays > 0 && $daysAfter >= $maxDays;
201205
}
202206

203207
protected function getNotificationsSummarized(array $notifications): array {

‎tests/php/Unit/Service/ReminderServiceTest.php‎

Lines changed: 131 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -140,29 +140,53 @@ public static function providerWillNotify(): array {
140140
],
141141
'now' => $now, 'daysBefore' => 1, 'daysBetween' => 1, 'max' => 0, false,
142142
],
143+
'no notification, scheduled for today, between = 0' => [
144+
[
145+
'first' => (clone $now)->setTime(0, 0),
146+
'last' => (clone $now)->setTime(0, 0),
147+
'total' => 1,
148+
],
149+
'now' => $now, 'daysBefore' => 1, 'daysBetween' => 0, 'max' => 5, false,
150+
],
151+
'no notification, scheduled for today, between = 1' => [
152+
[
153+
'first' => (clone $now)->setTime(0, 0),
154+
'last' => (clone $now)->setTime(0, 0),
155+
'total' => 1,
156+
],
157+
'now' => $now, 'daysBefore' => 1, 'daysBetween' => 1, 'max' => 5, false,
158+
],
159+
'no notification, scheduled for yesterday, between = 0' => [
160+
[
161+
'first' => (clone $now)->modify('-1 day')->setTime(0, 0),
162+
'last' => (clone $now)->modify('-1 day')->setTime(0, 0),
163+
'total' => 1,
164+
],
165+
'now' => $now, 'daysBefore' => 1, 'daysBetween' => 0, 'max' => 5, false,
166+
],
167+
'one notification, scheduled for yesterday, between = 1' => [
168+
[
169+
'first' => (clone $now)->modify('-1 day')->setTime(0, 0),
170+
'last' => (clone $now)->modify('-1 day')->setTime(0, 0),
171+
'total' => 1,
172+
],
173+
'now' => $now, 'daysBefore' => 1, 'daysBetween' => 1, 'max' => 5, true,
174+
],
143175
'one notification, should send' => [
144176
[
145177
'first' => (clone $now)->modify('-2 days'),
146178
'last' => (clone $now)->modify('-2 days'),
147179
'total' => 1,
148180
],
149-
'now' => $now, 'daysBefore' => 1, 'daysBetween' => 0, 'max' => 5, true,
181+
'now' => $now, 'daysBefore' => 1, 'daysBetween' => 1, 'max' => 5, true,
150182
],
151183
'one notification, should not send with daysBefore <= 0' => [
152184
[
153185
'first' => (clone $now)->modify('-1 day'),
154186
'last' => (clone $now)->modify('-1 day'),
155187
'total' => 1,
156188
],
157-
'now' => $now, 'daysBefore' => 1, 'daysBetween' => 0, 'max' => 5, false,
158-
],
159-
'two notifications, should not send with between === 1 and last === 1' => [
160-
[
161-
'first' => (clone $now)->modify('-3 days'),
162-
'last' => (clone $now)->modify('-1 day'),
163-
'total' => 2,
164-
],
165-
'now' => $now, 'daysBefore' => 1, 'daysBetween' => 1, 'max' => 5, false,
189+
'now' => $now, 'daysBefore' => 0, 'daysBetween' => 1, 'max' => 5, false,
166190
],
167191
'two notifications, should send with between === 1 and last === 2' => [
168192
[
@@ -194,7 +218,103 @@ public static function providerWillNotify(): array {
194218
'last' => (clone $now)->modify('-1 day'),
195219
'total' => 2,
196220
],
197-
'now' => $now, 'daysBefore' => 0, 'daysBetween' => 1, 'max' => 5, false,
221+
'now' => $now, 'daysBefore' => 0, 'daysBetween' => 0, 'max' => 5, false,
222+
],
223+
'one notification, exact daysBefore limit, should notify' => [
224+
[
225+
'first' => (clone $now)->modify('-1 day')->setTime(0, 0),
226+
'last' => (clone $now)->modify('-1 day')->setTime(0, 0),
227+
'total' => 1,
228+
],
229+
'now' => $now, 'daysBefore' => 1, 'daysBetween' => 1, 'max' => 5, true,
230+
],
231+
'two notifications, exact daysBetween limit, should notify' => [
232+
[
233+
'first' => (clone $now)->modify('-3 days'),
234+
'last' => (clone $now)->modify('-2 days')->setTime(0, 0),
235+
'total' => 2,
236+
],
237+
'now' => $now, 'daysBefore' => 1, 'daysBetween' => 2, 'max' => 5, true,
238+
],
239+
'no notifications, valid config but daysBetween = 0' => [
240+
[
241+
'first' => null,
242+
'last' => null,
243+
'total' => 0,
244+
],
245+
'now' => $now, 'daysBefore' => 1, 'daysBetween' => 0, 'max' => 5, false,
246+
],
247+
'inconsistent data: total > 0 but null dates' => [
248+
[
249+
'first' => null,
250+
'last' => null,
251+
'total' => 1,
252+
],
253+
'now' => $now, 'daysBefore' => 1, 'daysBetween' => 1, 'max' => 5, false,
254+
],
255+
'max = 0 means no limit, should send with valid config' => [
256+
[
257+
'first' => (clone $now)->modify('-2 days'),
258+
'last' => (clone $now)->modify('-2 days'),
259+
'total' => 1,
260+
],
261+
'now' => $now, 'daysBefore' => 1, 'daysBetween' => 1, 'max' => 0, true,
262+
],
263+
'max = 0, high total count, should still send' => [
264+
[
265+
'first' => (clone $now)->modify('-10 days'),
266+
'last' => (clone $now)->modify('-2 days'),
267+
'total' => 100,
268+
],
269+
'now' => $now, 'daysBefore' => 1, 'daysBetween' => 1, 'max' => 0, true,
270+
],
271+
'total exceeds max, should not send' => [
272+
[
273+
'first' => (clone $now)->modify('-5 days'),
274+
'last' => (clone $now)->modify('-2 days'),
275+
'total' => 6,
276+
],
277+
'now' => $now, 'daysBefore' => 1, 'daysBetween' => 1, 'max' => 5, false,
278+
],
279+
'notification today, should not send' => [
280+
[
281+
'first' => (clone $now)->setTime(0, 0),
282+
'last' => (clone $now)->setTime(0, 0),
283+
'total' => 1,
284+
],
285+
'now' => $now, 'daysBefore' => 1, 'daysBetween' => 1, 'max' => 5, false,
286+
],
287+
'multiple notifications, insufficient daysBetween' => [
288+
[
289+
'first' => (clone $now)->modify('-5 days'),
290+
'last' => (clone $now)->setTime(0, 0),
291+
'total' => 3,
292+
],
293+
'now' => $now, 'daysBefore' => 1, 'daysBetween' => 2, 'max' => 5, false,
294+
],
295+
'negative daysBefore, should not send' => [
296+
[
297+
'first' => (clone $now)->modify('-2 days'),
298+
'last' => (clone $now)->modify('-2 days'),
299+
'total' => 1,
300+
],
301+
'now' => $now, 'daysBefore' => -1, 'daysBetween' => 1, 'max' => 5, false,
302+
],
303+
'negative daysBetween, should not send' => [
304+
[
305+
'first' => (clone $now)->modify('-5 days'),
306+
'last' => (clone $now)->modify('-2 days'),
307+
'total' => 2,
308+
],
309+
'now' => $now, 'daysBefore' => 1, 'daysBetween' => -1, 'max' => 5, false,
310+
],
311+
'negative max acts as no limit, should send' => [
312+
[
313+
'first' => (clone $now)->modify('-3 days'),
314+
'last' => (clone $now)->modify('-2 days'),
315+
'total' => 10,
316+
],
317+
'now' => $now, 'daysBefore' => 1, 'daysBetween' => 1, 'max' => -1, true,
198318
],
199319
];
200320
}

0 commit comments

Comments
 (0)