From ee3b6321c14d15cd7dc02bd61f2ac93e71929b35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20Mari=C3=A9thoz?= Date: Thu, 19 Jul 2018 12:28:55 +0200 Subject: [PATCH] api: add missing transition conditions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Implement checkin, request, validate_request circulation policies using states conditions. (closes #28) Signed-off-by: Johnny Mariéthoz --- invenio_circulation/api.py | 72 +++++++++++++++++++--------- invenio_circulation/config.py | 10 +++- invenio_circulation/utils.py | 60 ++++++++++++++++++++++-- tests/conftest.py | 7 +++ tests/test_loan.py | 88 +++++++++++++++++++++++++---------- 5 files changed, 183 insertions(+), 54 deletions(-) diff --git a/invenio_circulation/api.py b/invenio_circulation/api.py index dbdf4df..d86477c 100644 --- a/invenio_circulation/api.py +++ b/invenio_circulation/api.py @@ -29,19 +29,24 @@ 'trigger': 'request', 'source': 'CREATED', 'dest': 'PENDING', - 'before': 'set_request_parameters' + 'before': 'set_request_parameters', + 'conditions': 'is_request_valid' }, { 'trigger': 'validate_request', 'source': 'PENDING', 'dest': 'ITEM_IN_TRANSIT', 'before': 'set_parameters', + 'conditions': 'is_validate_request_valid', 'unless': 'is_pickup_at_same_library' }, { 'trigger': 'validate_request', 'source': 'PENDING', 'dest': 'ITEM_AT_DESK', 'before': 'set_parameters', - 'conditions': 'is_pickup_at_same_library' + 'conditions': [ + 'is_validate_request_valid', + 'is_pickup_at_same_library' + ] }, { 'trigger': 'checkout', 'source': 'CREATED', @@ -52,7 +57,8 @@ 'trigger': 'checkin', 'source': 'ITEM_ON_LOAN', 'dest': 'ITEM_RETURNED', - 'before': 'set_parameters' + 'before': 'set_parameters', + 'conditions': 'is_checkin_valid' }] @@ -68,6 +74,24 @@ def __init__(self, data, model=None): initial=self['state'], finalize_event='save') + @property + def policies(self): + """.""" + return current_app.config.get('CIRCULATION_POLICIES') + + @classmethod + def export_diagram(cls, output_file): + """.""" + if not DIAGRAM_ENABLED: + warnings.warn('dependency not found, please install pygraphviz to ' + 'export the circulation state diagram.') + return False + m = GraphMachine(states=STATES, transitions=TRANSITIONS, + initial=STATES[0], show_conditions=True, + title='Circulation State Diagram') + m.get_graph().draw(output_file, prog='dot') + return True + def set_request_parameters(self, event): """.""" self.set_parameters(event) @@ -84,17 +108,20 @@ def set_parameters(self, event): self['transaction_date'] = params.get('transaction_date', datetime.now().isoformat()) + def save(self, event): + """.""" + if event.error: + raise event.error + else: + self['state'] = self.state + self.commit() + def is_pickup_at_same_library(self, event): """.""" item_location_pid = current_app.config.get( 'CIRCULATION_ITEM_LOCATION_RETRIEVER')(self['item_pid']) return self['pickup_location_pid'] == item_location_pid - @property - def policies(self): - """.""" - return current_app.config.get('CIRCULATION_POLICIES') - def is_checkout_valid(self, event): """.""" dates = self.policies['checkout'](**event.kwargs) @@ -103,23 +130,22 @@ def is_checkout_valid(self, event): self['start_date'], self['end_date'] = dates return True - def save(self, event): + def is_checkin_valid(self, event): """.""" - if event.error: - raise Exception(event.error) - else: - self['state'] = self.state - self.commit() + end_date = self.policies['checkin'](**event.kwargs) + if not end_date: + return False + self['end_date'] = end_date + return True - @classmethod - def export_diagram(cls, output_file): + def is_request_valid(self, event): """.""" - if not DIAGRAM_ENABLED: - warnings.warn('dependency not found, please install pygraphviz to ' - 'export the circulation state diagram.') + extra_params = self.policies['request'](**event.kwargs) + if not extra_params: return False - m = GraphMachine(states=STATES, transitions=TRANSITIONS, - initial=STATES[0], show_conditions=True, - title='Circulation State Diagram') - m.get_graph().draw(output_file, prog='dot') + self['pickup_location_pid'], self['request_expire_date'] = extra_params return True + + def is_validate_request_valid(self, event): + """.""" + return self.policies['validate_request'](**event.kwargs) diff --git a/invenio_circulation/config.py b/invenio_circulation/config.py index 32048db..1538d99 100644 --- a/invenio_circulation/config.py +++ b/invenio_circulation/config.py @@ -11,7 +11,8 @@ # TODO: This is an example file. Remove it if your package does not use any # extra configuration variables. -from .utils import is_checkout_valid, item_location_retriever +from .utils import is_checkin_valid, is_checkout_valid, is_request_valid, \ + is_request_validate_valid, item_location_retriever CIRCULATION_DEFAULT_VALUE = 'foobar' """Default value for the application.""" @@ -21,4 +22,9 @@ CIRCULATION_ITEM_LOCATION_RETRIEVER = item_location_retriever -CIRCULATION_POLICIES = dict(checkout=is_checkout_valid) +CIRCULATION_POLICIES = dict( + checkout=is_checkout_valid, + checkin=is_checkin_valid, + request=is_request_valid, + validate_request=is_request_validate_valid +) diff --git a/invenio_circulation/utils.py b/invenio_circulation/utils.py index fef8bbe..abb8618 100644 --- a/invenio_circulation/utils.py +++ b/invenio_circulation/utils.py @@ -10,6 +10,8 @@ from datetime import datetime, timedelta +from flask import current_app + def item_location_retriever(item_pid): """.""" @@ -18,14 +20,64 @@ def item_location_retriever(item_pid): def is_checkout_valid(transaction_user_pid, patron_pid, - item_pid, transaction_location_pid, transaction_date, + item_pid, start_date=None, end_date=None): """.""" if not start_date: - start_date = datetime.strptime(transaction_date, '%Y-%m-%d') + start_date = transaction_date + # 30 days by default + if not end_date: + end_date = datetime.strptime(start_date, '%Y-%m-%d') \ + + timedelta(days=30) + end_date = end_date.strftime('%Y-%m-%d') + assert datetime.strptime(start_date, '%Y-%m-%d') + assert datetime.strptime(end_date, '%Y-%m-%d') + return (start_date, end_date) + + +def is_checkin_valid(transaction_user_pid, + patron_pid, + transaction_location_pid, + transaction_date, + item_pid, + end_date=None): + """.""" + # 30 days by default if not end_date: - end_date = start_date + timedelta(days=30) - return (start_date.strftime('%Y-%m-%d'), end_date.strftime('%Y-%m-%d')) + end_date = datetime.strptime(transaction_date, '%Y-%m-%d') \ + + timedelta(days=30) + end_date = end_date.strftime('%Y-%m-%d') + assert datetime.strptime(end_date, '%Y-%m-%d') + return end_date + + +def is_request_valid(transaction_user_pid, + patron_pid, + transaction_location_pid, + transaction_date, + item_pid, + pickup_location_pid=None, + request_expire_date=None): + """.""" + # item location by default + if not pickup_location_pid: + pickup_location_pid = current_app.config.get( + 'CIRCULATION_ITEM_LOCATION_RETRIEVER')(item_pid) + # 30 days by default + if not request_expire_date: + request_expire_date = datetime.strptime(transaction_date, '%Y-%m-%d') \ + + timedelta(days=30) + request_expire_date = request_expire_date.strftime('%Y-%m-%d') + return (pickup_location_pid, request_expire_date) + + +def is_request_validate_valid(transaction_user_pid, + patron_pid, + item_pid, + transaction_location_pid, + transaction_date): + """.""" + return True diff --git a/tests/conftest.py b/tests/conftest.py index aa5c77f..2e82f97 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -24,6 +24,7 @@ from invenio_records.ext import InvenioRecords from sqlalchemy_utils.functions import create_database, database_exists +from invenio_circulation.api import Loan from invenio_circulation.ext import InvenioCirculation @@ -35,6 +36,12 @@ def instance_path(): shutil.rmtree(path) +@pytest.yield_fixture() +def loan(db): + """Minimal Loan object.""" + yield Loan.create({}) + + @pytest.fixture() def params(): """.""" diff --git a/tests/test_loan.py b/tests/test_loan.py index 1d1cb06..5eb9a3f 100644 --- a/tests/test_loan.py +++ b/tests/test_loan.py @@ -8,6 +8,8 @@ """Tests for loan states.""" +import pytest + from invenio_circulation.api import Loan @@ -23,18 +25,8 @@ def test_loan_creation(db, params): assert loan.state == 'ITEM_RETURNED' -def test_conditional_checkout(app, db, params): - """Test loan checkout and checkin.""" - loan = Loan.create({}) - loan.checkout(**params) - assert loan.state == 'ITEM_ON_LOAN' - assert loan['start_date'] == params.get('transaction_date') - - -def test_storing_loan_parameters(db, params): +def test_storing_loan_parameters(loan, db, params): """.""" - loan = Loan.create({}) - loan.checkout(**params) params['state'] = 'ITEM_ON_LOAN' @@ -43,9 +35,8 @@ def test_storing_loan_parameters(db, params): assert loan == params -def test_persist_loan(db, params): +def test_persist_loan(loan, db, params): """.""" - loan = Loan.create({}) loan.checkout(**params) db.session.commit() @@ -53,29 +44,76 @@ def test_persist_loan(db, params): assert loan.state == 'ITEM_ON_LOAN' -def test_validate_item_in_transit(app, db, params): +def test_validate_item_in_transit(loan, app, db, params): """.""" - loan = Loan.create({}) - params['pickup_location_pid'] = 'pickup_location_pid' - - loan.request(**params) + loan.request(**params, pickup_location_pid='pickup_location_pid') assert loan.state == 'PENDING' app.config['CIRCULATION_ITEM_LOCATION_RETRIEVER'] =\ lambda x: 'external_location_pid' - loan.validate_request() + loan.validate_request(**params) assert loan.state == 'ITEM_IN_TRANSIT' -def test_validate_item_at_desk(app, db, params): +def test_validate_item_at_desk(loan, app, db, params): """.""" - loan = Loan.create({}) - params['pickup_location_pid'] = 'pickup_location_pid' - - loan.request(**params) + loan.request(**params, pickup_location_pid='pickup_location_pid') assert loan.state == 'PENDING' app.config['CIRCULATION_ITEM_LOCATION_RETRIEVER'] =\ lambda x: 'pickup_location_pid' - loan.validate_request() + loan.validate_request(**params) assert loan.state == 'ITEM_AT_DESK' + + +def test_conditional_checkout(loan, app, db, params): + """Test checkout with some conditions.""" + loan.checkout(**params) + assert loan.state == 'ITEM_ON_LOAN' + assert loan['start_date'] == params.get('transaction_date') + + loan = Loan.create({}) + loan.checkout(**params, start_date='2020-01-01', end_date='2020-02-01') + assert loan.state == 'ITEM_ON_LOAN' + assert loan['start_date'] == '2020-01-01' + assert loan['end_date'] == '2020-02-01' + + loan = Loan.create({}) + with pytest.raises(ValueError): + loan.checkout(**params, start_date='2020-01-xx') + with pytest.raises(ValueError): + loan.checkout(**params, end_date='2020-01-xx') + + +def test_conditional_checkin(loan, app, db, params): + """Test checkin with some conditions.""" + loan.checkout(**params) + loan.checkin(**params) + assert loan.state == 'ITEM_RETURNED' + + loan = Loan.create({}) + loan.checkout(**params) + loan.checkin(**params, end_date='2020-02-01') + assert loan.state == 'ITEM_RETURNED' + assert loan['end_date'] == '2020-02-01' + + loan = Loan.create({}) + loan.checkout(**params) + with pytest.raises(ValueError): + loan.checkin(**params, end_date='2020-01-xx') + + +def test_conditional_request(loan, app, db, params): + """Test request with some conditions.""" + loan.request(**params, pickup_location_pid='my_pickup_location_pid', + request_expire_date='2020-02-01') + assert loan.state == 'PENDING' + loan['request_expire_date'] == '2020-02-01' + loan['pickup_location_pid'] == 'my_pickup_location_pid' + + loan = Loan.create({}) + app.config['CIRCULATION_ITEM_LOCATION_RETRIEVER'] =\ + lambda x: 'custom_pickup_location_pid' + loan.request(**params) + assert loan.state == 'PENDING' + loan['pickup_location_pid'] == 'custom_pickup_location_pid'