From 9b12d64a47c6956f3670d006ad22d194a77f14f9 Mon Sep 17 00:00:00 2001 From: Viktor Draban Date: Mon, 2 May 2022 13:00:37 +0300 Subject: [PATCH 1/7] CIRC-1513 remove due date schedule notices after checkin --- .../domain/OverdueFineService.java | 7 +- .../resources/CheckInByBarcodeResource.java | 7 + .../resources/CheckInProcessAdapter.java | 4 - .../resources/renewal/RenewalResource.java | 2 - .../loans/DueDateScheduledNoticesTests.java | 125 +++++++++++------- .../policies/PoliciesActivationFixture.java | 14 ++ .../domain/OverdueFineServiceTest.java | 13 +- 7 files changed, 104 insertions(+), 68 deletions(-) diff --git a/src/main/java/org/folio/circulation/domain/OverdueFineService.java b/src/main/java/org/folio/circulation/domain/OverdueFineService.java index 897506c3f3..360cff9c8c 100644 --- a/src/main/java/org/folio/circulation/domain/OverdueFineService.java +++ b/src/main/java/org/folio/circulation/domain/OverdueFineService.java @@ -26,7 +26,6 @@ import org.folio.circulation.infrastructure.storage.feesandfines.FeeFineRepository; import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; import org.folio.circulation.infrastructure.storage.loans.OverdueFinePolicyRepository; -import org.folio.circulation.infrastructure.storage.notices.ScheduledNoticesRepository; import org.folio.circulation.resources.context.RenewalContext; import org.folio.circulation.services.FeeFineFacade; import org.folio.circulation.services.support.CreateAccountCommand; @@ -42,7 +41,6 @@ public class OverdueFineService { private final ItemRepository itemRepository; private final FeeFineOwnerRepository feeFineOwnerRepository; private final FeeFineRepository feeFineRepository; - private final ScheduledNoticesRepository scheduledNoticesRepository; private final OverduePeriodCalculatorService overduePeriodCalculatorService; private final FeeFineFacade feeFineFacade; @@ -208,10 +206,7 @@ private CompletableFuture> createFeeFineRecord(Loan loan, .thenCompose(r -> r.after(this::lookupFeeFine)) .thenCompose(r -> r.after(this::lookupItemRelatedRecords)) .thenCompose(r -> r.after(this::lookupFeeFineOwner)) - .thenCompose(r -> r.after(this::createAccount)) - .thenCompose(r -> r.after(feeFineAction -> scheduledNoticesRepository - .deleteOverdueNotices(loan.getId()) - .thenApply(rs -> r))); + .thenCompose(r -> r.after(this::createAccount)); } private CompletableFuture> createAccount(CalculationParameters params) { diff --git a/src/main/java/org/folio/circulation/resources/CheckInByBarcodeResource.java b/src/main/java/org/folio/circulation/resources/CheckInByBarcodeResource.java index f4f8e92e89..2983d8b314 100644 --- a/src/main/java/org/folio/circulation/resources/CheckInByBarcodeResource.java +++ b/src/main/java/org/folio/circulation/resources/CheckInByBarcodeResource.java @@ -1,5 +1,6 @@ package org.folio.circulation.resources; +import static org.folio.circulation.domain.notice.schedule.TriggeringEvent.DUE_DATE; import static org.folio.circulation.domain.representations.CheckOutByBarcodeRequest.ITEM_BARCODE; import static org.folio.circulation.domain.validation.UserNotFoundValidator.refuseWhenLoggedInUserNotPresent; import static org.folio.circulation.support.ValidationErrorFailure.singleValidationError; @@ -14,6 +15,7 @@ import org.folio.circulation.infrastructure.storage.ConfigurationRepository; import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; import org.folio.circulation.infrastructure.storage.loans.LoanRepository; +import org.folio.circulation.infrastructure.storage.notices.ScheduledNoticesRepository; import org.folio.circulation.infrastructure.storage.requests.RequestQueueRepository; import org.folio.circulation.infrastructure.storage.requests.RequestRepository; import org.folio.circulation.infrastructure.storage.sessions.PatronActionSessionRepository; @@ -65,6 +67,8 @@ private void checkIn(RoutingContext routingContext) { final RequestScheduledNoticeService requestScheduledNoticeService = RequestScheduledNoticeService.using(clients); + final ScheduledNoticesRepository scheduledNoticesRepository = + ScheduledNoticesRepository.using(clients); final PatronActionSessionService patronActionSessionService = PatronActionSessionService.using(clients, @@ -94,6 +98,9 @@ private void checkIn(RoutingContext routingContext) { processAdapter::findSingleOpenLoan, CheckInContext::withLoan)) .thenComposeAsync(findLoanResult -> findLoanResult.combineAfter( processAdapter::checkInLoan, CheckInContext::withLoan)) + .thenComposeAsync(r -> r.combineAfter(checkInContext -> scheduledNoticesRepository + .deleteByLoanIdAndTriggeringEvent(checkInContext.getLoan().getId(), DUE_DATE), + (checkInContext, response) -> checkInContext)) .thenComposeAsync(checkInLoan -> checkInLoan.combineAfter( processAdapter::updateRequestQueue, CheckInContext::withRequestQueue)) .thenComposeAsync(updateRequestQueueResult -> updateRequestQueueResult.combineAfter( diff --git a/src/main/java/org/folio/circulation/resources/CheckInProcessAdapter.java b/src/main/java/org/folio/circulation/resources/CheckInProcessAdapter.java index 4d3a3593a8..990dd725b0 100644 --- a/src/main/java/org/folio/circulation/resources/CheckInProcessAdapter.java +++ b/src/main/java/org/folio/circulation/resources/CheckInProcessAdapter.java @@ -1,12 +1,10 @@ package org.folio.circulation.resources; import static java.util.concurrent.CompletableFuture.completedFuture; -import static org.folio.circulation.domain.RequestLevel.TITLE; import static org.folio.circulation.support.results.Result.succeeded; import java.util.UUID; import java.util.concurrent.CompletableFuture; -import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; import org.folio.circulation.domain.CheckInContext; @@ -29,7 +27,6 @@ import org.folio.circulation.infrastructure.storage.loans.LoanPolicyRepository; import org.folio.circulation.infrastructure.storage.loans.LoanRepository; import org.folio.circulation.infrastructure.storage.loans.OverdueFinePolicyRepository; -import org.folio.circulation.infrastructure.storage.notices.ScheduledNoticesRepository; import org.folio.circulation.infrastructure.storage.requests.RequestQueueRepository; import org.folio.circulation.infrastructure.storage.requests.RequestRepository; import org.folio.circulation.infrastructure.storage.users.AddressTypeRepository; @@ -108,7 +105,6 @@ public static CheckInProcessAdapter newInstance(Clients clients, new OverdueFinePolicyRepository(clients), itemRepository, new FeeFineOwnerRepository(clients), new FeeFineRepository(clients), - ScheduledNoticesRepository.using(clients), new OverduePeriodCalculatorService(new CalendarRepository(clients), new LoanPolicyRepository(clients)), new FeeFineFacade(clients)); diff --git a/src/main/java/org/folio/circulation/resources/renewal/RenewalResource.java b/src/main/java/org/folio/circulation/resources/renewal/RenewalResource.java index 0fef2631da..935dce6f89 100644 --- a/src/main/java/org/folio/circulation/resources/renewal/RenewalResource.java +++ b/src/main/java/org/folio/circulation/resources/renewal/RenewalResource.java @@ -76,7 +76,6 @@ import org.folio.circulation.infrastructure.storage.loans.LoanPolicyRepository; import org.folio.circulation.infrastructure.storage.loans.LoanRepository; import org.folio.circulation.infrastructure.storage.loans.OverdueFinePolicyRepository; -import org.folio.circulation.infrastructure.storage.notices.ScheduledNoticesRepository; import org.folio.circulation.infrastructure.storage.requests.RequestQueueRepository; import org.folio.circulation.infrastructure.storage.requests.RequestRepository; import org.folio.circulation.infrastructure.storage.users.UserRepository; @@ -246,7 +245,6 @@ private CompletableFuture> processFeesFinesForRegularRene itemRepository, new FeeFineOwnerRepository(clients), new FeeFineRepository(clients), - ScheduledNoticesRepository.using(clients), new OverduePeriodCalculatorService(new CalendarRepository(clients), new LoanPolicyRepository(clients)), new FeeFineFacade(clients)); diff --git a/src/test/java/api/loans/DueDateScheduledNoticesTests.java b/src/test/java/api/loans/DueDateScheduledNoticesTests.java index 1b4e651ee8..5ee5b0f41d 100644 --- a/src/test/java/api/loans/DueDateScheduledNoticesTests.java +++ b/src/test/java/api/loans/DueDateScheduledNoticesTests.java @@ -5,6 +5,8 @@ import static api.support.utl.BlockOverridesUtils.buildOkapiHeadersWithPermissions; import static api.support.utl.PatronNoticeTestHelper.verifyNumberOfScheduledNotices; import static java.time.ZoneOffset.UTC; +import static org.folio.circulation.domain.policy.Period.days; +import static org.folio.circulation.domain.policy.Period.hours; import static org.folio.circulation.support.json.JsonPropertyFetcher.getDateTimeProperty; import static org.hamcrest.CoreMatchers.hasItems; import static org.hamcrest.MatcherAssert.assertThat; @@ -40,42 +42,21 @@ class DueDateScheduledNoticesTests extends APITests { @Test void allDueDateNoticesShouldBeScheduledOnCheckoutWhenPolicyDefinesDueDateNoticeConfiguration() { UUID beforeTemplateId = UUID.randomUUID(); - Period beforePeriod = Period.days(2); - Period beforeRecurringPeriod = Period.hours(6); + Period beforePeriod = days(2); + Period beforeRecurringPeriod = hours(6); UUID uponAtTemplateId = UUID.randomUUID(); UUID afterTemplateId = UUID.randomUUID(); - Period afterPeriod = Period.days(3); - Period afterRecurringPeriod = Period.hours(4); - - JsonObject beforeDueDateNoticeConfiguration = new NoticeConfigurationBuilder() - .withTemplateId(beforeTemplateId) - .withDueDateEvent() - .withBeforeTiming(beforePeriod) - .recurring(beforeRecurringPeriod) - .sendInRealTime(true) - .create(); - JsonObject uponAtDueDateNoticeConfiguration = new NoticeConfigurationBuilder() - .withTemplateId(uponAtTemplateId) - .withDueDateEvent() - .withUponAtTiming() - .sendInRealTime(false) - .create(); - JsonObject afterDueDateNoticeConfiguration = new NoticeConfigurationBuilder() - .withTemplateId(afterTemplateId) - .withDueDateEvent() - .withAfterTiming(afterPeriod) - .recurring(afterRecurringPeriod) - .sendInRealTime(true) - .create(); + Period afterPeriod = days(3); + Period afterRecurringPeriod = hours(4); NoticePolicyBuilder noticePolicy = new NoticePolicyBuilder() .withName("Policy with due date notices") .withLoanNotices(Arrays.asList( - beforeDueDateNoticeConfiguration, - uponAtDueDateNoticeConfiguration, - afterDueDateNoticeConfiguration)); + createBeforeDueDateNoticeConfiguration(beforeTemplateId, beforePeriod, beforeRecurringPeriod, true), + createUponAtDueDateNoticeConfiguration(uponAtTemplateId, false), + createAfterDueDateNoticeConfiguration(afterTemplateId, afterPeriod, afterRecurringPeriod, true))); use(noticePolicy); IndividualResource smallAngryPlanet = itemsFixture.basedUponSmallAngryPlanet(); @@ -117,7 +98,7 @@ void checkOutSchedulesDifferentBeforeDueDateNotices() { UUID firstBeforeTemplateId = UUID.randomUUID(); Period firstBeforePeriod = Period.weeks(1); UUID secondBeforeTemplateId = UUID.randomUUID(); - Period secondBeforePeriod = Period.hours(12); + Period secondBeforePeriod = hours(12); JsonObject firstBeforeDueDateNoticeConfiguration = new NoticeConfigurationBuilder() .withTemplateId(firstBeforeTemplateId) @@ -211,14 +192,14 @@ void noNoticesShouldBeScheduledOnCheckOutWhenPolicyDoesNotDefineTimeBasedNotices @Test void noticesShouldBeRescheduledAfterRenewal() { UUID beforeTemplateId = UUID.randomUUID(); - Period beforePeriod = Period.days(2); - Period beforeRecurringPeriod = Period.hours(6); + Period beforePeriod = days(2); + Period beforeRecurringPeriod = hours(6); UUID uponAtTemplateId = UUID.randomUUID(); UUID afterTemplateId = UUID.randomUUID(); - Period afterPeriod = Period.days(3); - Period afterRecurringPeriod = Period.hours(4); + Period afterPeriod = days(3); + Period afterRecurringPeriod = hours(4); JsonObject beforeDueDateNoticeConfiguration = new NoticeConfigurationBuilder() .withTemplateId(beforeTemplateId) @@ -286,14 +267,14 @@ void noticesShouldBeRescheduledAfterRenewal() { @Test void noticesShouldBeRescheduledAfterRenewalOverride() { UUID beforeTemplateId = UUID.randomUUID(); - Period beforePeriod = Period.days(2); - Period beforeRecurringPeriod = Period.hours(6); + Period beforePeriod = days(2); + Period beforeRecurringPeriod = hours(6); UUID uponAtTemplateId = UUID.randomUUID(); UUID afterTemplateId = UUID.randomUUID(); - Period afterPeriod = Period.days(3); - Period afterRecurringPeriod = Period.hours(4); + Period afterPeriod = days(3); + Period afterRecurringPeriod = hours(4); JsonObject beforeDueDateNoticeConfiguration = new NoticeConfigurationBuilder() .withTemplateId(beforeTemplateId) @@ -369,14 +350,14 @@ void noticesShouldBeRescheduledAfterRenewalOverride() { @Test void noticesShouldBeRescheduledAfterRecall() { UUID beforeTemplateId = UUID.randomUUID(); - Period beforePeriod = Period.days(2); - Period beforeRecurringPeriod = Period.hours(6); + Period beforePeriod = days(2); + Period beforeRecurringPeriod = hours(6); UUID uponAtTemplateId = UUID.randomUUID(); UUID afterTemplateId = UUID.randomUUID(); - Period afterPeriod = Period.days(3); - Period afterRecurringPeriod = Period.hours(4); + Period afterPeriod = days(3); + Period afterRecurringPeriod = hours(4); JsonObject beforeDueDateNoticeConfiguration = new NoticeConfigurationBuilder() .withTemplateId(beforeTemplateId) @@ -454,14 +435,14 @@ void noticesShouldBeRescheduledAfterRecall() { @Test void noticesShouldBeRescheduledAfterManualDueDateChange() { UUID beforeTemplateId = UUID.randomUUID(); - Period beforePeriod = Period.days(2); - Period beforeRecurringPeriod = Period.hours(6); + Period beforePeriod = days(2); + Period beforeRecurringPeriod = hours(6); UUID uponAtTemplateId = UUID.randomUUID(); UUID afterTemplateId = UUID.randomUUID(); - Period afterPeriod = Period.days(3); - Period afterRecurringPeriod = Period.hours(4); + Period afterPeriod = days(3); + Period afterRecurringPeriod = hours(4); JsonObject beforeDueDateNoticeConfiguration = new NoticeConfigurationBuilder() .withTemplateId(beforeTemplateId) @@ -529,4 +510,58 @@ void noticesShouldBeRescheduledAfterManualDueDateChange() { verifyNumberOfScheduledNotices(6); } + @Test + void allDueDateNoticesShouldBeDeletedAfternCheckin() { + IndividualResource smallAngryPlanet = itemsFixture.basedUponSmallAngryPlanet(); + NoticePolicyBuilder noticePolicy = new NoticePolicyBuilder() + .withName("Policy with due date notices") + .withLoanNotices(Arrays.asList( + createBeforeDueDateNoticeConfiguration(UUID.randomUUID(), days(2), hours(6), true), + createUponAtDueDateNoticeConfiguration(UUID.randomUUID(), false), + createAfterDueDateNoticeConfiguration(UUID.randomUUID(), days(3), hours(4), false))); + + useWithNoOverdueFine(noticePolicy); + + checkOutFixture.checkOutByBarcode( + smallAngryPlanet, + usersFixture.steve()); + + verifyNumberOfScheduledNotices(3); + checkInFixture.checkInByBarcode(smallAngryPlanet); + verifyNumberOfScheduledNotices(0); + } + + private JsonObject createBeforeDueDateNoticeConfiguration(UUID templateId, Period beforePeriod, + Period beforeRecurringPeriod, boolean realTime) { + + return new NoticeConfigurationBuilder() + .withTemplateId(templateId) + .withDueDateEvent() + .withBeforeTiming(beforePeriod) + .recurring(beforeRecurringPeriod) + .sendInRealTime(realTime) + .create(); + } + + private JsonObject createUponAtDueDateNoticeConfiguration(UUID templateId, boolean realTime) { + return new NoticeConfigurationBuilder() + .withTemplateId(templateId) + .withDueDateEvent() + .withUponAtTiming() + .sendInRealTime(realTime) + .create(); + } + + private JsonObject createAfterDueDateNoticeConfiguration(UUID templateId, Period afterPeriod, + Period afterRecurringPeriod, boolean realTime) { + + return new NoticeConfigurationBuilder() + .withTemplateId(templateId) + .withDueDateEvent() + .withAfterTiming(afterPeriod) + .recurring(afterRecurringPeriod) + .sendInRealTime(realTime) + .create(); + } + } diff --git a/src/test/java/api/support/fixtures/policies/PoliciesActivationFixture.java b/src/test/java/api/support/fixtures/policies/PoliciesActivationFixture.java index 305197fe69..bbddd790cb 100644 --- a/src/test/java/api/support/fixtures/policies/PoliciesActivationFixture.java +++ b/src/test/java/api/support/fixtures/policies/PoliciesActivationFixture.java @@ -159,6 +159,20 @@ public void use(NoticePolicyBuilder noticePolicy) { .noticePolicy(noticePoliciesFixture.create(noticePolicy))); } + /** + * This method uses notice policy, canCirculateRolling loan policy, + * allowAllRequestPolicy request policy, + * noOverdueFine overdue fine policy from + * the loanPolicyBuilder. + * + * @param noticePolicy - notice policy. + */ + public void useWithNoOverdueFine(NoticePolicyBuilder noticePolicy) { + use(defaultRollingPolicies() + .overduePolicy(overdueFinePoliciesFixture.noOverdueFine()) + .noticePolicy(noticePoliciesFixture.create(noticePolicy))); + } + public void useLostItemPolicy(UUID lostItemFeePolicyId) { final PoliciesToActivate policies = defaultRollingPolicies().build(); diff --git a/src/test/java/org/folio/circulation/domain/OverdueFineServiceTest.java b/src/test/java/org/folio/circulation/domain/OverdueFineServiceTest.java index e50da33388..bf3ea3144d 100644 --- a/src/test/java/org/folio/circulation/domain/OverdueFineServiceTest.java +++ b/src/test/java/org/folio/circulation/domain/OverdueFineServiceTest.java @@ -35,7 +35,6 @@ import org.folio.circulation.infrastructure.storage.feesandfines.FeeFineRepository; import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; import org.folio.circulation.infrastructure.storage.loans.OverdueFinePolicyRepository; -import org.folio.circulation.infrastructure.storage.notices.ScheduledNoticesRepository; import org.folio.circulation.infrastructure.storage.users.UserRepository; import org.folio.circulation.resources.context.RenewalContext; import org.folio.circulation.services.FeeFineFacade; @@ -99,7 +98,6 @@ class OverdueFineServiceTest { private FeeFineOwnerRepository feeFineOwnerRepository; private FeeFineRepository feeFineRepository; private FeeFineActionRepository feeFineActionRepository; - private ScheduledNoticesRepository scheduledNoticesRepository; private ServicePointRepository servicePointRepository; @BeforeEach @@ -112,7 +110,6 @@ public void setUp() { overduePeriodCalculatorService = mock(OverduePeriodCalculatorService.class); UserRepository userRepository = mock(UserRepository.class); feeFineActionRepository = mock(FeeFineActionRepository.class); - scheduledNoticesRepository = mock(ScheduledNoticesRepository.class); servicePointRepository = mock(ServicePointRepository.class); FeeFineService feeFineService = mock(FeeFineService.class); FeeFineFacade feeFineFacade = new FeeFineFacade(accountRepository, @@ -120,10 +117,8 @@ public void setUp() { userRepository, servicePointRepository, feeFineService); - overdueFineService = new OverdueFineService( - overdueFinePolicyRepository, itemRepository, - feeFineOwnerRepository, feeFineRepository, scheduledNoticesRepository, - overduePeriodCalculatorService, feeFineFacade); + overdueFineService = new OverdueFineService(overdueFinePolicyRepository, itemRepository, + feeFineOwnerRepository, feeFineRepository, overduePeriodCalculatorService, feeFineFacade); when(userRepository.getUser(any(String.class))).thenReturn( completedFuture(succeeded(LOGGED_IN_USER))); @@ -540,8 +535,6 @@ void shouldDeleteOverdueNoticesWhenFeeFineRecordCreated( when(accountRepository.create(any())).thenReturn(completedFuture(succeeded(createAccount(correctOverdueFine)))); when(feeFineActionRepository.create(any())) .thenReturn(completedFuture(succeeded(createFeeFineAction()))); - when(scheduledNoticesRepository.deleteOverdueNotices(any())) - .thenReturn(completedFuture(succeeded(null))); when(servicePointRepository.getServicePointById(CHECK_IN_SERVICE_POINT_ID.toString())) .thenReturn(completedFuture(succeeded(createServicePoint()))); @@ -557,8 +550,6 @@ void shouldDeleteOverdueNoticesWhenFeeFineRecordCreated( overdueFineService.createOverdueFineIfNecessary(context, LOGGED_IN_USER_ID).get(); } - - verify(scheduledNoticesRepository, times(1)).deleteOverdueNotices(any()); } private RenewalContext createRenewalContext(Loan loan) { From dcdbc807ccbba5ae45d2f2f8747518b3b057e26d Mon Sep 17 00:00:00 2001 From: Viktor Draban Date: Mon, 2 May 2022 16:39:41 +0300 Subject: [PATCH 2/7] CIRC-1513 add nullpoint check --- .../resources/CheckInByBarcodeResource.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/folio/circulation/resources/CheckInByBarcodeResource.java b/src/main/java/org/folio/circulation/resources/CheckInByBarcodeResource.java index 2983d8b314..d709a60aae 100644 --- a/src/main/java/org/folio/circulation/resources/CheckInByBarcodeResource.java +++ b/src/main/java/org/folio/circulation/resources/CheckInByBarcodeResource.java @@ -7,6 +7,7 @@ import org.folio.circulation.domain.CheckInContext; import org.folio.circulation.domain.Item; +import org.folio.circulation.domain.Loan; import org.folio.circulation.domain.notice.schedule.RequestScheduledNoticeService; import org.folio.circulation.domain.notice.session.PatronActionSessionService; import org.folio.circulation.domain.representations.CheckInByBarcodeRequest; @@ -31,6 +32,8 @@ import io.vertx.ext.web.Router; import io.vertx.ext.web.RoutingContext; +import java.util.Optional; + public class CheckInByBarcodeResource extends Resource { public CheckInByBarcodeResource(HttpClient client) { super(client); @@ -98,8 +101,14 @@ private void checkIn(RoutingContext routingContext) { processAdapter::findSingleOpenLoan, CheckInContext::withLoan)) .thenComposeAsync(findLoanResult -> findLoanResult.combineAfter( processAdapter::checkInLoan, CheckInContext::withLoan)) - .thenComposeAsync(r -> r.combineAfter(checkInContext -> scheduledNoticesRepository - .deleteByLoanIdAndTriggeringEvent(checkInContext.getLoan().getId(), DUE_DATE), + .thenComposeAsync(r -> r.combineAfter(checkInContext -> { + String loanId = Optional.ofNullable(checkInContext) + .map(CheckInContext::getLoan) + .map(Loan::getId) + .orElse(null); + return scheduledNoticesRepository + .deleteByLoanIdAndTriggeringEvent(loanId, DUE_DATE); + }, (checkInContext, response) -> checkInContext)) .thenComposeAsync(checkInLoan -> checkInLoan.combineAfter( processAdapter::updateRequestQueue, CheckInContext::withRequestQueue)) From fe4b7c9e7aa58b202402b28ed94c645258a2d57a Mon Sep 17 00:00:00 2001 From: Viktor Draban Date: Mon, 2 May 2022 17:25:06 +0300 Subject: [PATCH 3/7] CIRC-1513 the check is extended --- .../folio/circulation/resources/CheckInByBarcodeResource.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/folio/circulation/resources/CheckInByBarcodeResource.java b/src/main/java/org/folio/circulation/resources/CheckInByBarcodeResource.java index d709a60aae..056eeb7a21 100644 --- a/src/main/java/org/folio/circulation/resources/CheckInByBarcodeResource.java +++ b/src/main/java/org/folio/circulation/resources/CheckInByBarcodeResource.java @@ -104,6 +104,7 @@ private void checkIn(RoutingContext routingContext) { .thenComposeAsync(r -> r.combineAfter(checkInContext -> { String loanId = Optional.ofNullable(checkInContext) .map(CheckInContext::getLoan) + .filter(Loan::isClosed) .map(Loan::getId) .orElse(null); return scheduledNoticesRepository From 44f471ebb547a51dac77f1a727bebe6859fbfd4c Mon Sep 17 00:00:00 2001 From: Viktor Draban Date: Tue, 3 May 2022 11:26:50 +0300 Subject: [PATCH 4/7] CIRC-1513 fix test --- .../api/loans/DueDateScheduledNoticesProcessingTests.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/test/java/api/loans/DueDateScheduledNoticesProcessingTests.java b/src/test/java/api/loans/DueDateScheduledNoticesProcessingTests.java index 51cf85bf92..317d8e7d0f 100644 --- a/src/test/java/api/loans/DueDateScheduledNoticesProcessingTests.java +++ b/src/test/java/api/loans/DueDateScheduledNoticesProcessingTests.java @@ -583,14 +583,6 @@ void allDueDateNoticesAreDiscardedWhenLoanIsClosed() { checkInFixture.checkInByBarcode(item); - verifyNumberOfScheduledNotices(5); - - var processingTime = AFTER_RECURRING_PERIOD - .plusDate(AFTER_PERIOD.plusDate(dueDate)) - .plusSeconds(1); - - scheduledNoticeProcessingClient.runLoanNoticesProcessing(processingTime); - verifyNumberOfScheduledNotices(0); verifyNumberOfSentNotices(0); verifyNumberOfPublishedEvents(NOTICE, 0); From 487cd486749f80f53f241e46778db93ed71c294b Mon Sep 17 00:00:00 2001 From: Viktor Draban Date: Tue, 3 May 2022 12:08:14 +0300 Subject: [PATCH 5/7] CIRC-1513 remove test due to notices are being deleted separately --- .../domain/OverdueFineServiceTest.java | 41 ------------------- 1 file changed, 41 deletions(-) diff --git a/src/test/java/org/folio/circulation/domain/OverdueFineServiceTest.java b/src/test/java/org/folio/circulation/domain/OverdueFineServiceTest.java index bf3ea3144d..bfd13bd804 100644 --- a/src/test/java/org/folio/circulation/domain/OverdueFineServiceTest.java +++ b/src/test/java/org/folio/circulation/domain/OverdueFineServiceTest.java @@ -511,47 +511,6 @@ void shouldNotCreateFeeFineForRenewalWhenShouldForgiveOverdueFine( verifyNoInteractions(accountRepository); } - @ParameterizedTest - @MethodSource("testParameters") - void shouldDeleteOverdueNoticesWhenFeeFineRecordCreated( - Boolean renewal, Boolean dueDateChangedByRecall, Double overdueFine, String overdueFineInterval, - Double maxOverdueFine, Double overdueRecallFine, String overdueRecallFineInterval, - Double maxOverdueRecallFine, Integer periodCalculatorResult, Double correctOverdueFine) - throws ExecutionException, InterruptedException { - Loan loan = createLoan(overdueFine, overdueFineInterval, overdueRecallFine, - overdueRecallFineInterval, maxOverdueFine, maxOverdueRecallFine, - dueDateChangedByRecall); - - when(overdueFinePolicyRepository.findOverdueFinePolicyForLoan(any())) - .thenReturn(completedFuture(succeeded(loan))); - when(overduePeriodCalculatorService.getMinutes(any(), any())) - .thenReturn(completedFuture(succeeded(periodCalculatorResult))); - when(itemRepository.fetchItemRelatedRecords(any())) - .thenReturn(completedFuture(succeeded(createItem()))); - when(feeFineOwnerRepository.findOwnerForServicePoint(SERVICE_POINT_ID.toString())) - .thenReturn(completedFuture(succeeded(createFeeFineOwner()))); - when(feeFineRepository.getFeeFine(FEE_FINE_TYPE, true)) - .thenReturn(completedFuture(succeeded(createFeeFine()))); - when(accountRepository.create(any())).thenReturn(completedFuture(succeeded(createAccount(correctOverdueFine)))); - when(feeFineActionRepository.create(any())) - .thenReturn(completedFuture(succeeded(createFeeFineAction()))); - when(servicePointRepository.getServicePointById(CHECK_IN_SERVICE_POINT_ID.toString())) - .thenReturn(completedFuture(succeeded(createServicePoint()))); - - if (renewal) { - RenewalContext context = createRenewalContext(loan); - - overdueFineService.createOverdueFineIfNecessary(context).get(); - } - else { - CheckInContext context = new CheckInContext( - CheckInByBarcodeRequest.from(createCheckInByBarcodeRequest()).value()) - .withLoan(loan); - - overdueFineService.createOverdueFineIfNecessary(context, LOGGED_IN_USER_ID).get(); - } - } - private RenewalContext createRenewalContext(Loan loan) { return create(loan, new JsonObject(), LOGGED_IN_USER_ID); } From 9921b8c144afd8f93ae3bd32f0f00b933e80155b Mon Sep 17 00:00:00 2001 From: Viktor Draban Date: Tue, 3 May 2022 13:36:23 +0300 Subject: [PATCH 6/7] CIRC-1513 extract method --- .../resources/CheckInByBarcodeResource.java | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/folio/circulation/resources/CheckInByBarcodeResource.java b/src/main/java/org/folio/circulation/resources/CheckInByBarcodeResource.java index 056eeb7a21..634b44446c 100644 --- a/src/main/java/org/folio/circulation/resources/CheckInByBarcodeResource.java +++ b/src/main/java/org/folio/circulation/resources/CheckInByBarcodeResource.java @@ -4,6 +4,7 @@ import static org.folio.circulation.domain.representations.CheckOutByBarcodeRequest.ITEM_BARCODE; import static org.folio.circulation.domain.validation.UserNotFoundValidator.refuseWhenLoggedInUserNotPresent; import static org.folio.circulation.support.ValidationErrorFailure.singleValidationError; +import static org.folio.circulation.support.results.ResultBinding.mapResult; import org.folio.circulation.domain.CheckInContext; import org.folio.circulation.domain.Item; @@ -101,16 +102,8 @@ private void checkIn(RoutingContext routingContext) { processAdapter::findSingleOpenLoan, CheckInContext::withLoan)) .thenComposeAsync(findLoanResult -> findLoanResult.combineAfter( processAdapter::checkInLoan, CheckInContext::withLoan)) - .thenComposeAsync(r -> r.combineAfter(checkInContext -> { - String loanId = Optional.ofNullable(checkInContext) - .map(CheckInContext::getLoan) - .filter(Loan::isClosed) - .map(Loan::getId) - .orElse(null); - return scheduledNoticesRepository - .deleteByLoanIdAndTriggeringEvent(loanId, DUE_DATE); - }, - (checkInContext, response) -> checkInContext)) + .thenApplyAsync(mapResult(checkInContext -> + removeDueDateNoticesForClosedLoan(checkInContext, scheduledNoticesRepository))) .thenComposeAsync(checkInLoan -> checkInLoan.combineAfter( processAdapter::updateRequestQueue, CheckInContext::withRequestQueue)) .thenComposeAsync(updateRequestQueueResult -> updateRequestQueueResult.combineAfter( @@ -151,4 +144,17 @@ private ValidationErrorFailure errorWhenInIncorrectStatus(Item item) { return singleValidationError(message, ITEM_BARCODE, item.getBarcode()); } + + private CheckInContext removeDueDateNoticesForClosedLoan(CheckInContext context, + ScheduledNoticesRepository repository) { + + String loanId = Optional.ofNullable(context) + .map(CheckInContext::getLoan) + .filter(Loan::isClosed) + .map(Loan::getId) + .orElse(null); + + repository.deleteByLoanIdAndTriggeringEvent(loanId, DUE_DATE); + return context; + } } From 50ab13e81fd6425ef9e7d6a919e60390920eccac Mon Sep 17 00:00:00 2001 From: Viktor Draban Date: Tue, 3 May 2022 15:22:35 +0300 Subject: [PATCH 7/7] CIRC-1513 refactore test --- .../loans/DueDateScheduledNoticesTests.java | 166 ++++-------------- .../fixtures/ConfigurationsFixture.java | 36 ++++ 2 files changed, 66 insertions(+), 136 deletions(-) diff --git a/src/test/java/api/loans/DueDateScheduledNoticesTests.java b/src/test/java/api/loans/DueDateScheduledNoticesTests.java index 5ee5b0f41d..349e0f01e9 100644 --- a/src/test/java/api/loans/DueDateScheduledNoticesTests.java +++ b/src/test/java/api/loans/DueDateScheduledNoticesTests.java @@ -54,9 +54,11 @@ void allDueDateNoticesShouldBeScheduledOnCheckoutWhenPolicyDefinesDueDateNoticeC NoticePolicyBuilder noticePolicy = new NoticePolicyBuilder() .withName("Policy with due date notices") .withLoanNotices(Arrays.asList( - createBeforeDueDateNoticeConfiguration(beforeTemplateId, beforePeriod, beforeRecurringPeriod, true), - createUponAtDueDateNoticeConfiguration(uponAtTemplateId, false), - createAfterDueDateNoticeConfiguration(afterTemplateId, afterPeriod, afterRecurringPeriod, true))); + configurationsFixture.createBeforeDueDateNoticeConfiguration( + beforeTemplateId, beforePeriod, beforeRecurringPeriod, true), + configurationsFixture.createUponAtDueDateNoticeConfiguration(uponAtTemplateId, false), + configurationsFixture.createAfterDueDateNoticeConfiguration( + afterTemplateId, afterPeriod, afterRecurringPeriod, true))); use(noticePolicy); IndividualResource smallAngryPlanet = itemsFixture.basedUponSmallAngryPlanet(); @@ -201,33 +203,14 @@ void noticesShouldBeRescheduledAfterRenewal() { Period afterPeriod = days(3); Period afterRecurringPeriod = hours(4); - JsonObject beforeDueDateNoticeConfiguration = new NoticeConfigurationBuilder() - .withTemplateId(beforeTemplateId) - .withDueDateEvent() - .withBeforeTiming(beforePeriod) - .recurring(beforeRecurringPeriod) - .sendInRealTime(true) - .create(); - JsonObject uponAtDueDateNoticeConfiguration = new NoticeConfigurationBuilder() - .withTemplateId(uponAtTemplateId) - .withDueDateEvent() - .withUponAtTiming() - .sendInRealTime(false) - .create(); - JsonObject afterDueDateNoticeConfiguration = new NoticeConfigurationBuilder() - .withTemplateId(afterTemplateId) - .withDueDateEvent() - .withAfterTiming(afterPeriod) - .recurring(afterRecurringPeriod) - .sendInRealTime(true) - .create(); - NoticePolicyBuilder noticePolicy = new NoticePolicyBuilder() .withName("Policy with due date notices") .withLoanNotices(Arrays.asList( - beforeDueDateNoticeConfiguration, - uponAtDueDateNoticeConfiguration, - afterDueDateNoticeConfiguration)); + configurationsFixture.createBeforeDueDateNoticeConfiguration( + beforeTemplateId, beforePeriod, beforeRecurringPeriod, true), + configurationsFixture.createUponAtDueDateNoticeConfiguration(uponAtTemplateId, false), + configurationsFixture.createAfterDueDateNoticeConfiguration( + afterTemplateId, afterPeriod, afterRecurringPeriod, true))); use(noticePolicy); @@ -276,33 +259,14 @@ void noticesShouldBeRescheduledAfterRenewalOverride() { Period afterPeriod = days(3); Period afterRecurringPeriod = hours(4); - JsonObject beforeDueDateNoticeConfiguration = new NoticeConfigurationBuilder() - .withTemplateId(beforeTemplateId) - .withDueDateEvent() - .withBeforeTiming(beforePeriod) - .recurring(beforeRecurringPeriod) - .sendInRealTime(true) - .create(); - JsonObject uponAtDueDateNoticeConfiguration = new NoticeConfigurationBuilder() - .withTemplateId(uponAtTemplateId) - .withDueDateEvent() - .withUponAtTiming() - .sendInRealTime(false) - .create(); - JsonObject afterDueDateNoticeConfiguration = new NoticeConfigurationBuilder() - .withTemplateId(afterTemplateId) - .withDueDateEvent() - .withAfterTiming(afterPeriod) - .recurring(afterRecurringPeriod) - .sendInRealTime(true) - .create(); - NoticePolicyBuilder noticePolicy = new NoticePolicyBuilder() .withName("Policy with due date notices") .withLoanNotices(Arrays.asList( - beforeDueDateNoticeConfiguration, - uponAtDueDateNoticeConfiguration, - afterDueDateNoticeConfiguration)); + configurationsFixture.createBeforeDueDateNoticeConfiguration( + beforeTemplateId, beforePeriod, beforeRecurringPeriod, true), + configurationsFixture.createUponAtDueDateNoticeConfiguration(uponAtTemplateId, false), + configurationsFixture.createAfterDueDateNoticeConfiguration( + afterTemplateId, afterPeriod, afterRecurringPeriod, true))); LoanPolicyBuilder loanPolicy = new LoanPolicyBuilder() .withName("Not renewable") @@ -359,33 +323,14 @@ void noticesShouldBeRescheduledAfterRecall() { Period afterPeriod = days(3); Period afterRecurringPeriod = hours(4); - JsonObject beforeDueDateNoticeConfiguration = new NoticeConfigurationBuilder() - .withTemplateId(beforeTemplateId) - .withDueDateEvent() - .withBeforeTiming(beforePeriod) - .recurring(beforeRecurringPeriod) - .sendInRealTime(true) - .create(); - JsonObject uponAtDueDateNoticeConfiguration = new NoticeConfigurationBuilder() - .withTemplateId(uponAtTemplateId) - .withDueDateEvent() - .withUponAtTiming() - .sendInRealTime(false) - .create(); - JsonObject afterDueDateNoticeConfiguration = new NoticeConfigurationBuilder() - .withTemplateId(afterTemplateId) - .withDueDateEvent() - .withAfterTiming(afterPeriod) - .recurring(afterRecurringPeriod) - .sendInRealTime(true) - .create(); - NoticePolicyBuilder noticePolicy = new NoticePolicyBuilder() .withName("Policy with due date notices") .withLoanNotices(Arrays.asList( - beforeDueDateNoticeConfiguration, - uponAtDueDateNoticeConfiguration, - afterDueDateNoticeConfiguration)); + configurationsFixture.createBeforeDueDateNoticeConfiguration( + beforeTemplateId, beforePeriod, beforeRecurringPeriod, true), + configurationsFixture.createUponAtDueDateNoticeConfiguration(uponAtTemplateId, false), + configurationsFixture.createAfterDueDateNoticeConfiguration( + afterTemplateId, afterPeriod, afterRecurringPeriod, true))); LoanPolicyBuilder loanPolicy = new LoanPolicyBuilder() .withName("Can Circulate Rolling for recall") @@ -444,33 +389,14 @@ void noticesShouldBeRescheduledAfterManualDueDateChange() { Period afterPeriod = days(3); Period afterRecurringPeriod = hours(4); - JsonObject beforeDueDateNoticeConfiguration = new NoticeConfigurationBuilder() - .withTemplateId(beforeTemplateId) - .withDueDateEvent() - .withBeforeTiming(beforePeriod) - .recurring(beforeRecurringPeriod) - .sendInRealTime(true) - .create(); - JsonObject uponAtDueDateNoticeConfiguration = new NoticeConfigurationBuilder() - .withTemplateId(uponAtTemplateId) - .withDueDateEvent() - .withUponAtTiming() - .sendInRealTime(false) - .create(); - JsonObject afterDueDateNoticeConfiguration = new NoticeConfigurationBuilder() - .withTemplateId(afterTemplateId) - .withDueDateEvent() - .withAfterTiming(afterPeriod) - .recurring(afterRecurringPeriod) - .sendInRealTime(true) - .create(); - NoticePolicyBuilder noticePolicy = new NoticePolicyBuilder() .withName("Policy with due date notices") .withLoanNotices(Arrays.asList( - beforeDueDateNoticeConfiguration, - uponAtDueDateNoticeConfiguration, - afterDueDateNoticeConfiguration)); + configurationsFixture.createBeforeDueDateNoticeConfiguration( + beforeTemplateId, beforePeriod, beforeRecurringPeriod, true), + configurationsFixture.createUponAtDueDateNoticeConfiguration(uponAtTemplateId, false), + configurationsFixture.createAfterDueDateNoticeConfiguration( + afterTemplateId, afterPeriod, afterRecurringPeriod, true))); use(noticePolicy); @@ -516,9 +442,11 @@ void allDueDateNoticesShouldBeDeletedAfternCheckin() { NoticePolicyBuilder noticePolicy = new NoticePolicyBuilder() .withName("Policy with due date notices") .withLoanNotices(Arrays.asList( - createBeforeDueDateNoticeConfiguration(UUID.randomUUID(), days(2), hours(6), true), - createUponAtDueDateNoticeConfiguration(UUID.randomUUID(), false), - createAfterDueDateNoticeConfiguration(UUID.randomUUID(), days(3), hours(4), false))); + configurationsFixture.createBeforeDueDateNoticeConfiguration( + UUID.randomUUID(), days(2), hours(6), true), + configurationsFixture.createUponAtDueDateNoticeConfiguration(UUID.randomUUID(), false), + configurationsFixture.createAfterDueDateNoticeConfiguration( + UUID.randomUUID(), days(3), hours(4), false))); useWithNoOverdueFine(noticePolicy); @@ -530,38 +458,4 @@ void allDueDateNoticesShouldBeDeletedAfternCheckin() { checkInFixture.checkInByBarcode(smallAngryPlanet); verifyNumberOfScheduledNotices(0); } - - private JsonObject createBeforeDueDateNoticeConfiguration(UUID templateId, Period beforePeriod, - Period beforeRecurringPeriod, boolean realTime) { - - return new NoticeConfigurationBuilder() - .withTemplateId(templateId) - .withDueDateEvent() - .withBeforeTiming(beforePeriod) - .recurring(beforeRecurringPeriod) - .sendInRealTime(realTime) - .create(); - } - - private JsonObject createUponAtDueDateNoticeConfiguration(UUID templateId, boolean realTime) { - return new NoticeConfigurationBuilder() - .withTemplateId(templateId) - .withDueDateEvent() - .withUponAtTiming() - .sendInRealTime(realTime) - .create(); - } - - private JsonObject createAfterDueDateNoticeConfiguration(UUID templateId, Period afterPeriod, - Period afterRecurringPeriod, boolean realTime) { - - return new NoticeConfigurationBuilder() - .withTemplateId(templateId) - .withDueDateEvent() - .withAfterTiming(afterPeriod) - .recurring(afterRecurringPeriod) - .sendInRealTime(realTime) - .create(); - } - } diff --git a/src/test/java/api/support/fixtures/ConfigurationsFixture.java b/src/test/java/api/support/fixtures/ConfigurationsFixture.java index 9bb49a6b45..3ffcb0400b 100644 --- a/src/test/java/api/support/fixtures/ConfigurationsFixture.java +++ b/src/test/java/api/support/fixtures/ConfigurationsFixture.java @@ -2,7 +2,10 @@ import java.util.UUID; +import api.support.builders.NoticeConfigurationBuilder; import api.support.http.ResourceClient; +import io.vertx.core.json.JsonObject; +import org.folio.circulation.domain.policy.Period; public class ConfigurationsFixture { private final ResourceClient client; @@ -37,4 +40,37 @@ public void configureTlrFeature(boolean isTlrFeatureEnabled, UUID confirmationT isTlrFeatureEnabled, confirmationTemplateId, cancellationTemplateId, expirationTemplateId)) .getId(); } + + public JsonObject createBeforeDueDateNoticeConfiguration(UUID templateId, Period beforePeriod, + Period beforeRecurringPeriod, boolean realTime) { + + return new NoticeConfigurationBuilder() + .withTemplateId(templateId) + .withDueDateEvent() + .withBeforeTiming(beforePeriod) + .recurring(beforeRecurringPeriod) + .sendInRealTime(realTime) + .create(); + } + + public JsonObject createUponAtDueDateNoticeConfiguration(UUID templateId, boolean realTime) { + return new NoticeConfigurationBuilder() + .withTemplateId(templateId) + .withDueDateEvent() + .withUponAtTiming() + .sendInRealTime(realTime) + .create(); + } + + public JsonObject createAfterDueDateNoticeConfiguration(UUID templateId, Period afterPeriod, + Period afterRecurringPeriod, boolean realTime) { + + return new NoticeConfigurationBuilder() + .withTemplateId(templateId) + .withDueDateEvent() + .withAfterTiming(afterPeriod) + .recurring(afterRecurringPeriod) + .sendInRealTime(realTime) + .create(); + } }