From aabedbcac40d43cf700ce10b996d8bc21b84d16b Mon Sep 17 00:00:00 2001 From: graham-dds Date: Mon, 30 Dec 2019 13:43:59 -0500 Subject: [PATCH 1/2] Make PoP start and end dates inclusive. Also removes the clock class. Makes PoP date ranges inclusive such that a task order with: - a start date on or after the current date and - an end date on or before the current date should be considered valid. This commit also removes the Clock class. This class had two methods as shortcuts for common uses of pendlum functions. But it wasn't being used in very many places, and it took up about the same space as from pendulum import today() ... today(tz="UTC").date() If we want to add this back in, it might be a good idea to extend it for other time functions we have sprinkled around, like the random date functions in our tests --- atst/domain/authnid/crl/util.py | 2 +- atst/models/task_order.py | 12 ++++++------ atst/utils/clock.py | 11 ----------- tests/models/test_task_order.py | 23 +++++++++++++++++------ 4 files changed, 24 insertions(+), 24 deletions(-) delete mode 100644 atst/utils/clock.py diff --git a/atst/domain/authnid/crl/util.py b/atst/domain/authnid/crl/util.py index 7668a71f..6cc22d6a 100644 --- a/atst/domain/authnid/crl/util.py +++ b/atst/domain/authnid/crl/util.py @@ -276,7 +276,7 @@ def existing_crl_modification_time(crl): prev_time = os.path.getmtime(crl) buffered = prev_time + MODIFIED_TIME_BUFFER mod_time = prev_time if pendulum.now().timestamp() < buffered else buffered - dt = pendulum.from_timestamp(mod_time, tz="GMT") + dt = pendulum.from_timestamp(mod_time, tz="UTC") return dt.format("ddd, DD MMM YYYY HH:mm:ss zz") else: diff --git a/atst/models/task_order.py b/atst/models/task_order.py index eba34147..6cf4df0f 100644 --- a/atst/models/task_order.py +++ b/atst/models/task_order.py @@ -9,7 +9,7 @@ from atst.models.base import Base import atst.models.types as types import atst.models.mixins as mixins from atst.models.attachment import Attachment -from atst.utils.clock import Clock +from pendulum import today class Status(Enum): @@ -117,17 +117,17 @@ class TaskOrder(Base, mixins.TimestampsMixin): @property def status(self): - today = Clock.today() + todays_date = today(tz="UTC").date() if not self.is_completed and not self.is_signed: return Status.DRAFT elif self.is_completed and not self.is_signed: return Status.UNSIGNED - elif today < self.start_date: + elif todays_date < self.start_date: return Status.UPCOMING - elif today >= self.end_date: + elif todays_date > self.end_date: return Status.EXPIRED - elif self.start_date <= today < self.end_date: + elif self.start_date <= todays_date <= self.end_date: return Status.ACTIVE @property @@ -141,7 +141,7 @@ class TaskOrder(Base, mixins.TimestampsMixin): @property def days_to_expiration(self): if self.end_date: - return (self.end_date - Clock.today()).days + return (self.end_date - today(tz="UTC").date()).days @property def total_obligated_funds(self): diff --git a/atst/utils/clock.py b/atst/utils/clock.py deleted file mode 100644 index 1ac4455e..00000000 --- a/atst/utils/clock.py +++ /dev/null @@ -1,11 +0,0 @@ -import pendulum - - -class Clock(object): - @classmethod - def today(cls, tz="UTC"): - return pendulum.today(tz=tz).date() - - @classmethod - def now(cls, tz="UTC"): - return pendulum.now(tz=tz) diff --git a/tests/models/test_task_order.py b/tests/models/test_task_order.py index 91fee15b..27512bf1 100644 --- a/tests/models/test_task_order.py +++ b/tests/models/test_task_order.py @@ -87,17 +87,28 @@ class TestTaskOrderStatus: @patch("atst.models.TaskOrder.is_completed", new_callable=PropertyMock) @patch("atst.models.TaskOrder.is_signed", new_callable=PropertyMock) def test_active_status(self, is_signed, is_completed, start_date, end_date): - # Given that I have a signed TO and today is within its start_date and end_date - today = pendulum.today().date() - to = TaskOrderFactory.create() + today = pendulum.today(tz="UTC").date() - start_date.return_value = today.subtract(days=1) - end_date.return_value = today.add(days=1) is_signed.return_value = True is_completed.return_value = True + # Given that I have a signed TO and today is within its start_date and end_date + to_1 = TaskOrderFactory.create() + start_date.return_value = today.subtract(days=1) + end_date.return_value = today.add(days=1) + # Its status should be active - assert to.status == Status.ACTIVE + assert to_1.status == Status.ACTIVE + + # A period of performance's start and end dates are inclusive, so a TO + # should be active on its start and end dates + to_2 = TaskOrderFactory.create() + start_date.return_value = today + end_date.return_value = today + is_signed.return_value = True + is_completed.return_value = True + + assert to_2.status == Status.ACTIVE @patch("atst.models.TaskOrder.end_date", new_callable=PropertyMock) @patch("atst.models.TaskOrder.start_date", new_callable=PropertyMock) From 46ed1f0e719161fe2990fd1b517b4252cdaf6fcc Mon Sep 17 00:00:00 2001 From: graham-dds Date: Mon, 30 Dec 2019 13:45:39 -0500 Subject: [PATCH 2/2] Remove / refactor TO class properties This commit removes properties that weren't be used anywhere in the code base. It also refactors two properties to use sum() with a generator comprehension instead of a for loop. --- atst/models/task_order.py | 41 ++++----------------------------------- 1 file changed, 4 insertions(+), 37 deletions(-) diff --git a/atst/models/task_order.py b/atst/models/task_order.py index 6cf4df0f..36ba905e 100644 --- a/atst/models/task_order.py +++ b/atst/models/task_order.py @@ -83,26 +83,10 @@ class TaskOrder(Base, mixins.TimestampsMixin): def is_active(self): return self.status == Status.ACTIVE - @property - def is_upcoming(self): - return self.status == Status.UPCOMING - @property def is_expired(self): return self.status == Status.EXPIRED - @property - def is_unsigned(self): - return self.status == Status.UNSIGNED - - @property - def has_begun(self): - return self.start_date is not None and Clock.today() >= self.start_date - - @property - def has_ended(self): - return self.start_date is not None and Clock.today() >= self.end_date - @property def clins_are_completed(self): return all([len(self.clins), (clin.is_completed for clin in self.clins)]) @@ -145,30 +129,13 @@ class TaskOrder(Base, mixins.TimestampsMixin): @property def total_obligated_funds(self): - total = 0 - for clin in self.clins: - if clin.obligated_amount is not None: - total += clin.obligated_amount - return total + return sum( + (clin.obligated_amount for clin in self.clins if clin.obligated_amount) + ) @property def total_contract_amount(self): - total = 0 - for clin in self.clins: - if clin.total_amount is not None: - total += clin.total_amount - return total - - @property - # TODO delete when we delete task_order_review flow - def budget(self): - return 100000 - - @property - def balance(self): - # TODO: fix task order -- reimplement using CLINs - # Faked for display purposes - return 50 + return sum((clin.total_amount for clin in self.clins if clin.total_amount)) @property def invoiced_funds(self):