Ensure open-ended book holds do not expire

Also rename date and datetime fields to reflect their content. Date fields end with `on`
and DateTime fields end with `at`.

README.md

@@ -7,4 +7,32 @@ Protean example implementation
 | Source          | [DDD by Examples - Library](https://github.com/ddd-by-examples/library) |
 | Pattern         | CQRS                                                                    |
 | Protean Version | 0.12.0                                                                  |
-| Coverage        | [![codecov](https://codecov.io/github/proteanhq/library-cqrs/graph/badge.svg?token=onIFcl4Dg5)](https://codecov.io/github/proteanhq/library-cqrs)|
+| Coverage        | [![codecov](https://codecov.io/github/proteanhq/library-cqrs/graph/badge.svg?token=onIFcl4Dg5)](https://codecov.io/github/proteanhq/library-cqrs)|
+
+## Contributing
+
+- Clone this repository:
+
+`git clone [email protected]:proteanhq/library-cqrs.git`
+
+- Set up and activate Python virtual environment:
+
+`python3 -m venv .venv`
+
+`source .venv/bin/activate`
+
+- Install [poetry](https://python-poetry.org/docs/#installation)
+
+- Install dependencies:
+
+`poetry install`
+
+- Install pre-commit hooks:
+
+`pre-commit install`
+
+## Running Tests
+
+- Basic: `make test`
+
+- With coverage: `make test-cov`

src/lending/checkout_service.py

@@ -52,7 +52,7 @@ def __call__(self):
                 patron_type=self.patron.patron_type,
                 book_id=self.book.id,
                 branch_id=self.branch_id,
-                checkout_date=checkout.checkout_date,
-                due_date=checkout.due_date,
+                checked_out_at=checkout.checked_out_at,
+                due_on=checkout.due_on,
             )
         )

src/lending/daily_sheet_service.py

@@ -19,12 +19,12 @@ def _expire_holds(self):
             for hold in patron.holds:
                 if (
                     hold.status == HoldStatus.ACTIVE.value
-                    and hold.expiry_date < date.today()
+                    and hold.expires_on < date.today()
                 ):
                     patron.expire_hold(hold.id)
 
     def _overdue_checkouts(self):
         for patron in self.patrons:
             for checkout in patron.checkouts:
-                if checkout.due_date < date.today():
+                if checkout.due_on < date.today():
                     checkout.overdue()

src/lending/holding_service.py

@@ -63,7 +63,7 @@ def patron_cannot_not_have_more_than_two_overdue_checkouts_in_branch(self):
         overdue_checkouts_in_branch = [
             checkout
             for checkout in self.patron.checkouts
-            if checkout.due_date < date.today() and checkout.branch_id == self.branch_id
+            if checkout.due_on < date.today() and checkout.branch_id == self.branch_id
         ]
         if len(overdue_checkouts_in_branch) > 2:
             raise ValidationError(
@@ -74,14 +74,26 @@ def patron_cannot_not_have_more_than_two_overdue_checkouts_in_branch(self):
                 }
             )
 
+    @invariant.post
+    def open_holds_do_not_have_expiry_date(self):
+        if self.hold_type == HoldType.OPEN_ENDED:
+            if self.patron.holds[-1].expires_on:
+                raise ValidationError(
+                    {"expires_on": ["Open-ended holds do not have an expiry date"]}
+                )
+
     def __call__(self):
+        expires_on = None
+        if self.hold_type == HoldType.CLOSED_ENDED:
+            expires_on = date.today() + timedelta(days=7)
+
         hold = Hold(
             book_id=self.book.id,
             branch_id=self.branch_id,
             hold_type=self.hold_type.value,
             status=HoldStatus.ACTIVE.value,
-            request_date=datetime.now(),
-            expiry_date=date.today() + timedelta(days=7),
+            requested_at=datetime.now(),
+            expires_on=expires_on,
         )
         self.patron.add_holds(hold)
 
@@ -93,7 +105,7 @@ def __call__(self):
                 book_id=hold.book_id,
                 branch_id=hold.branch_id,
                 hold_type=hold.hold_type,
-                request_date=hold.request_date,
-                expiry_date=hold.expiry_date,
+                requested_at=hold.requested_at,
+                expires_on=hold.expires_on,
             )
         )

src/lending/patron.py

@@ -37,8 +37,8 @@ class HoldExpired:
     branch_id = Identifier(required=True)
     book_id = Identifier(required=True)
     hold_type = String(required=True)
-    request_date = DateTime(required=True)
-    expiry_date = DateTime(required=True)
+    requested_at = DateTime(required=True)
+    expires_on = Date()
 
 
 @lending.event(part_of="Patron")
@@ -51,8 +51,8 @@ class HoldPlaced:
     book_id = Identifier(required=True)
     branch_id = Identifier(required=True)
     hold_type = String(required=True)
-    request_date = DateTime(required=True)
-    expiry_date = DateTime(required=True)
+    requested_at = DateTime(required=True)
+    expires_on = Date()
 
 
 @lending.event(part_of="Patron")
@@ -65,8 +65,8 @@ class HoldCancelled:
     branch_id = Identifier(required=True)
     book_id = Identifier(required=True)
     hold_type = String(required=True)
-    request_date = DateTime(required=True)
-    expiry_date = DateTime(required=True)
+    requested_at = DateTime(required=True)
+    expires_on = Date()
 
 
 @lending.entity(part_of="Patron")
@@ -78,8 +78,8 @@ class Hold:
     branch_id = Identifier(required=True)
     hold_type = String(max_length=12, default=HoldType.CLOSED_ENDED.value)
     status = String(max_length=11, default=HoldStatus.ACTIVE.value)
-    request_date = DateTime(required=True)
-    expiry_date = Date(required=True)
+    requested_at = DateTime(required=True)
+    expires_on = Date()
 
     def checkout(self):
         self.status = HoldStatus.CHECKED_OUT.value
@@ -94,13 +94,13 @@ def expire(self):
                 branch_id=self.branch_id,
                 book_id=self.book_id,
                 hold_type=self.hold_type,
-                request_date=self.request_date,
-                expiry_date=self.expiry_date,
+                requested_at=self.requested_at,
+                expires_on=self.expires_on,
             )
         )
 
     def cancel(self):
-        if self.status == HoldStatus.EXPIRED.value or self.expiry_date < date.today():
+        if self.status == HoldStatus.EXPIRED.value or self.expires_on < date.today():
             raise ValidationError({"expired_hold": ["Cannot cancel expired holds"]})
 
         if self.status == HoldStatus.CHECKED_OUT.value:
@@ -116,8 +116,8 @@ def cancel(self):
                 book_id=self.book_id,
                 branch_id=self.branch_id,
                 hold_type=self.hold_type,
-                request_date=self.request_date,
-                expiry_date=self.expiry_date,
+                requested_at=self.requested_at,
+                expires_on=self.expires_on,
             )
         )
 
@@ -136,8 +136,8 @@ class BookCheckedOut:
     patron_type = String(required=True)
     book_id = Identifier(required=True)
     branch_id = Identifier(required=True)
-    checkout_date = DateTime(required=True)
-    due_date = Date(required=True)
+    checked_out_at = DateTime(required=True)
+    due_on = Date(required=True)
 
 
 @lending.event(part_of="Patron")
@@ -148,9 +148,9 @@ class BookReturned:
     patron_type = String(required=True)
     book_id = Identifier(required=True)
     branch_id = Identifier(required=True)
-    checkout_date = DateTime(required=True)
-    due_date = Date(required=True)
-    return_date = DateTime(required=True)
+    checked_out_at = DateTime(required=True)
+    due_on = Date(required=True)
+    returned_at = DateTime(required=True)
 
 
 @lending.event(part_of="Patron")
@@ -161,8 +161,8 @@ class BookOverdue:
     patron_type = String(required=True)
     book_id = Identifier(required=True)
     branch_id = Identifier(required=True)
-    checkout_date = DateTime(required=True)
-    due_date = Date(required=True)
+    checked_out_at = DateTime(required=True)
+    due_on = Date(required=True)
 
 
 @lending.entity(part_of="Patron")
@@ -172,28 +172,28 @@ class Checkout:
 
     book_id = Identifier(required=True)
     branch_id = Identifier(required=True)
-    checkout_date = DateTime(required=True, default=utc_now)
+    checked_out_at = DateTime(required=True, default=utc_now)
     status = String(max_length=11, default=CheckoutStatus.ACTIVE.value)
-    due_date = Date(
+    due_on = Date(
         required=True,
         default=lambda: date.today()
         + timedelta(days=lending.config["custom"]["CHECKOUT_PERIOD"]),
     )
-    return_date = DateTime()
+    returned_at = DateTime()
 
     def return_(self):
         self.status = CheckoutStatus.RETURNED.value
-        self.return_date = datetime.now()
+        self.returned_at = datetime.now()
 
         self.raise_(
             BookReturned(
                 patron_id=self._owner.id,
                 patron_type=self._owner.patron_type,
                 book_id=self.book_id,
                 branch_id=self.branch_id,
-                checkout_date=self.checkout_date,
-                due_date=self.due_date,
-                return_date=self.return_date,
+                checked_out_at=self.checked_out_at,
+                due_on=self.due_on,
+                returned_at=self.returned_at,
             )
         )
 
@@ -206,8 +206,8 @@ def overdue(self):
                 patron_type=self._owner.patron_type,
                 book_id=self.book_id,
                 branch_id=self.branch_id,
-                checkout_date=self.checkout_date,
-                due_date=self.due_date,
+                checked_out_at=self.checked_out_at,
+                due_on=self.due_on,
             )
         )
 

tests/lending/bdd/checkouts/step_defs/checkout_steps.py

@@ -63,7 +63,7 @@ def patron_with_checkout(regular_patron, book):
 @given("the book is overdue")
 def mark_checkout_overdue():
     patron = g.current_user
-    patron.checkouts[0].due_date = patron.checkouts[0].due_date - timedelta(days=1)
+    patron.checkouts[0].due_on = patron.checkouts[0].due_on - timedelta(days=1)
     patron.checkouts[0].status = CheckoutStatus.OVERDUE.value
 
 
@@ -89,7 +89,7 @@ def system_has_overdue_checkouts():
         ]
     )
     # Manually expire a checkout
-    patron1.checkouts[0].due_date = date.today() - timedelta(days=1)
+    patron1.checkouts[0].due_on = date.today() - timedelta(days=1)
 
     patron2.add_checkouts(
         Checkout(
@@ -98,7 +98,7 @@ def system_has_overdue_checkouts():
         )
     )
     # Manually exipre a checkout
-    patron2.checkouts[0].due_date = date.today() - timedelta(days=1)
+    patron2.checkouts[0].due_on = date.today() - timedelta(days=1)
 
     g.current_patrons = [patron1, patron2]
 
@@ -138,7 +138,7 @@ def confirm_checkout_book():
 @then(cfparse("the checkout has a validity of {validity_days_config}"))
 def confirm_checkout_expiry(validity_days_config):
     checkout = g.current_user.checkouts[0]
-    assert checkout.due_date == date.today() + timedelta(
+    assert checkout.due_on == date.today() + timedelta(
         days=current_domain.config["custom"][validity_days_config]
     )
 

tests/lending/bdd/holds/features/open_and_close_ended_holds.feature

@@ -5,6 +5,7 @@ Feature: Handle Open-ended and Closed-ended Holds
     And a researcher patron is logged in
     When the patron places an open-ended hold
     Then the hold is successfully placed
+    And the hold does not have an expiry date
 
   Scenario: Regular patron tries to place an open-ended hold
     Given a circulating book is available

tests/lending/bdd/holds/step_defs/hold_steps.py

@@ -71,7 +71,7 @@ def closed_ended_hold_placed():
 
 @given("the hold has reached its expiry date")
 def hold_expired():
-    g.current_user.holds[0].expiry_date = date.today() - timedelta(days=1)
+    g.current_user.holds[0].expires_on = date.today() - timedelta(days=1)
 
 
 @given("patron has fewer than five holds")
@@ -100,7 +100,7 @@ def patron_with_expired_hold(patron, book):
     g.current_book = book
 
     place_hold(g.current_user, book, "1", HoldType.CLOSED_ENDED)()
-    g.current_user.holds[0].expiry_date = date.today() - timedelta(days=1)
+    g.current_user.holds[0].expires_on = date.today() - timedelta(days=1)
 
 
 @given("a patron has a hold that has been checked out")
@@ -230,3 +230,8 @@ def check_hold_cancellation_rejected():
     assert "HoldCancelled" not in [
         event.__class__.__name__ for event in g.current_user._events
     ]
+
+
+@then("the hold does not have an expiry date")
+def confirm_no_expiry_date():
+    assert g.current_user.holds[0].expires_on is None

tests/lending/conftest.py

@@ -71,27 +71,27 @@ def researcher_patron(patron):
 
 @pytest.fixture
 def overdue_checkouts_patron(patron):
-    checkout_date1 = fake.date_between(start_date="-90d", end_date="-61d")
-    checkout_date2 = fake.date_between(start_date="-80d", end_date="-71d")
-    checkout_date3 = fake.date_between(start_date="-85d", end_date="-75d")
+    checkout_date1 = fake.date_time_between(start_date="-90d", end_date="-61d")
+    checkout_date2 = fake.date_time_between(start_date="-80d", end_date="-71d")
+    checkout_date3 = fake.date_time_between(start_date="-85d", end_date="-75d")
     patron.checkouts = [
         lending.Checkout(
             branch_id="1",
             book_id=fake.uuid4(),
-            checkout_date=checkout_date1,
-            due_date=checkout_date1 + timedelta(days=60),
+            checked_out_at=checkout_date1,
+            due_on=checkout_date1.date() + timedelta(days=60),
         ),
         lending.Checkout(
             branch_id="1",
             book_id=fake.uuid4(),
-            checkout_date=checkout_date2,
-            due_date=checkout_date2 + timedelta(days=60),
+            checked_out_at=checkout_date2,
+            due_on=checkout_date2.date() + timedelta(days=60),
         ),
         lending.Checkout(
             branch_id="1",
             book_id=fake.uuid4(),
-            checkout_date=checkout_date3,
-            due_date=checkout_date3 + timedelta(days=60),
+            checked_out_at=checkout_date3,
+            due_on=checkout_date3.date() + timedelta(days=60),
         ),
     ]
     return patron

tests/lending/tdd/patron/test_checkout_entity.py

@@ -11,5 +11,5 @@ def test_checkout_entity_element_type():
 def test_checkout_entity_declared_fields():
     assert all(
         field_name in declared_fields(Checkout)
-        for field_name in ["id", "book_id", "checkout_date", "due_date"]
+        for field_name in ["id", "book_id", "checked_out_at", "due_on"]
     )