From 34502966a42075e8a9ac602c66ad47d433781603 Mon Sep 17 00:00:00 2001 From: Antti Rae Date: Mon, 30 Sep 2024 15:51:25 +0300 Subject: [PATCH] fix open ended permit end time calculation. ParkingPermit end_time was wrong after renewal in some cases when incrementing end time by a month. refs: PV-852 --- .github/workflows/ci.yml | 1 + parking_permits/models/parking_permit.py | 2 +- .../tests/models/test_parking_permit.py | 104 ++++++++++++++++-- parking_permits/tests/test_utils.py | 22 ++++ parking_permits/utils.py | 19 +++- 5 files changed, 138 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 23d875f7..aabb4342 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,6 +45,7 @@ jobs: - name: Install required Ubuntu packages run: | + sudo apt-get update sudo apt-get install gdal-bin - name: Install PyPI dependencies diff --git a/parking_permits/models/parking_permit.py b/parking_permits/models/parking_permit.py index 7ba8d316..ac49f6bf 100644 --- a/parking_permits/models/parking_permit.py +++ b/parking_permits/models/parking_permit.py @@ -807,7 +807,7 @@ def renew_open_ended_permit(self): if self.contract_type != ContractType.OPEN_ENDED: raise ValueError("This permit is not open-ended so cannot be renewed") self.end_time = increment_end_time( - self.end_time or self.current_period_end_time(), months=1 + self.start_time, self.end_time or self.current_period_end_time(), months=1 ) self.save() diff --git a/parking_permits/tests/models/test_parking_permit.py b/parking_permits/tests/models/test_parking_permit.py index 4dca225c..285ac2f6 100644 --- a/parking_permits/tests/models/test_parking_permit.py +++ b/parking_permits/tests/models/test_parking_permit.py @@ -1113,6 +1113,96 @@ def test_extend_permit(self): self.assertEqual(permit.month_count, 4) self.assertEqual(permit.end_time.date(), date(2024, 7, 3)) + def test_renew_parking_permit_multiple_renews_middle_of_month(self): + permit = ParkingPermitFactory( + status=ParkingPermitStatus.VALID, + contract_type=ContractType.OPEN_ENDED, + start_time=timezone.make_aware(datetime(2024, 1, 10, 21, 0), pytz.UTC), + end_time=timezone.make_aware( + datetime(2024, 2, 9, 20, 59, 59, 999999), pytz.UTC + ), + month_count=1, + ) + + with freeze_time("2024-1-15"): + permit.renew_open_ended_permit() + permit.refresh_from_db() + + self.assertEqual( + permit.end_time.isoformat(), + "2024-03-09T21:59:59.999999+00:00", + ) + + self.assertEqual( + timezone.localtime(permit.end_time).isoformat(), + "2024-03-09T23:59:59.999999+02:00", + ) + + with freeze_time("2024-2-15"): + permit.renew_open_ended_permit() + permit.refresh_from_db() + + self.assertEqual( + permit.end_time.isoformat(), + "2024-04-09T20:59:59.999999+00:00", + ) + + self.assertEqual( + timezone.localtime(permit.end_time).isoformat(), + "2024-04-09T23:59:59.999999+03:00", + ) + + def test_renew_parking_permit_multiple_renews_end_of_month(self): + permit = ParkingPermitFactory( + status=ParkingPermitStatus.VALID, + contract_type=ContractType.OPEN_ENDED, + start_time=timezone.make_aware(datetime(2024, 1, 1, 21, 0), pytz.UTC), + end_time=timezone.make_aware( + datetime(2024, 1, 31, 20, 59, 59, 999999), pytz.UTC + ), + month_count=1, + ) + + with freeze_time("2024-1-24"): + permit.renew_open_ended_permit() + permit.refresh_from_db() + + self.assertEqual( + permit.end_time.isoformat(), + "2024-02-29T21:59:59.999999+00:00", + ) + + self.assertEqual( + timezone.localtime(permit.end_time).isoformat(), + "2024-02-29T23:59:59.999999+02:00", + ) + + with freeze_time("2024-2-24"): + permit.renew_open_ended_permit() + permit.refresh_from_db() + self.assertEqual( + permit.end_time.isoformat(), + "2024-03-31T20:59:59.999999+00:00", + ) + + self.assertEqual( + timezone.localtime(permit.end_time).isoformat(), + "2024-03-31T23:59:59.999999+03:00", + ) + with freeze_time("2024-3-24"): + permit.renew_open_ended_permit() + permit.refresh_from_db() + + self.assertEqual( + permit.end_time.isoformat(), + "2024-04-30T20:59:59.999999+00:00", + ) + + self.assertEqual( + timezone.localtime(permit.end_time).isoformat(), + "2024-04-30T23:59:59.999999+03:00", + ) + @freeze_time("2024-1-28") def test_renew_parking_permit(self): permit = ParkingPermitFactory( @@ -1137,13 +1227,13 @@ def test_renew_parking_permit(self): "2024-02-29T23:59:59.999999+02:00", ) - @freeze_time("2024-3-7") + @freeze_time("2024-3-31") def test_renew_parking_permit_dst_to_summer(self): permit = ParkingPermitFactory( status=ParkingPermitStatus.VALID, contract_type=ContractType.OPEN_ENDED, start_time=timezone.make_aware(datetime(2024, 1, 1, 12, 0), pytz.UTC), - end_time=timezone.make_aware(datetime(2024, 3, 10, 21, 59), pytz.UTC), + end_time=timezone.make_aware(datetime(2024, 3, 31, 20, 59), pytz.UTC), month_count=1, ) @@ -1153,12 +1243,12 @@ def test_renew_parking_permit_dst_to_summer(self): # should be 23:59 Helsinki time i.e. UTC + 2 hours self.assertEqual( permit.end_time.isoformat(), - "2024-04-10T20:59:59.999999+00:00", + "2024-04-30T20:59:59.999999+00:00", ) self.assertEqual( timezone.localtime(permit.end_time).isoformat(), - "2024-04-10T23:59:59.999999+03:00", + "2024-04-30T23:59:59.999999+03:00", ) @freeze_time("2024-10-15") @@ -1167,7 +1257,7 @@ def test_renew_parking_permit_dst_to_winter(self): status=ParkingPermitStatus.VALID, contract_type=ContractType.OPEN_ENDED, start_time=timezone.make_aware(datetime(2024, 1, 1, 12, 0), pytz.UTC), - end_time=timezone.make_aware(datetime(2024, 10, 12, 20, 59), pytz.UTC), + end_time=timezone.make_aware(datetime(2024, 10, 31, 20, 59), pytz.UTC), month_count=1, ) @@ -1177,12 +1267,12 @@ def test_renew_parking_permit_dst_to_winter(self): # should be 23:59 Helsinki time i.e. UTC + 2 hours self.assertEqual( permit.end_time.isoformat(), - "2024-11-12T21:59:59.999999+00:00", + "2024-11-30T21:59:59.999999+00:00", ) self.assertEqual( timezone.localtime(permit.end_time).isoformat(), - "2024-11-12T23:59:59.999999+02:00", + "2024-11-30T23:59:59.999999+02:00", ) @freeze_time("2024-3-15 9:00+02:00") diff --git a/parking_permits/tests/test_utils.py b/parking_permits/tests/test_utils.py index f8116399..3a0a5183 100644 --- a/parking_permits/tests/test_utils.py +++ b/parking_permits/tests/test_utils.py @@ -16,10 +16,32 @@ diff_months_floor, find_next_date, flatten_dict, + get_last_day_of_month, get_model_diff, ) +def test_get_last_day_of_month(): + assert get_last_day_of_month(datetime(2024, 1, 2, 12, 12, 00)) == 31 + assert get_last_day_of_month(datetime(2023, 2, 2, 12, 12, 00)) == 28 + assert get_last_day_of_month(datetime(2024, 3, 2, 12, 12, 00)) == 31 + assert get_last_day_of_month(datetime(2024, 4, 2, 12, 12, 00)) == 30 + assert get_last_day_of_month(datetime(2024, 5, 2, 12, 12, 00)) == 31 + assert get_last_day_of_month(datetime(2024, 6, 2, 12, 12, 00)) == 30 + assert get_last_day_of_month(datetime(2024, 7, 2, 12, 12, 00)) == 31 + assert get_last_day_of_month(datetime(2024, 8, 2, 12, 12, 00)) == 31 + assert get_last_day_of_month(datetime(2024, 9, 2, 12, 12, 00)) == 30 + assert get_last_day_of_month(datetime(2024, 10, 2, 12, 12, 00)) == 31 + assert get_last_day_of_month(datetime(2024, 11, 2, 12, 12, 00)) == 30 + assert get_last_day_of_month(datetime(2024, 12, 2, 12, 12, 00)) == 31 + + # leap year + assert get_last_day_of_month(datetime(2024, 2, 2, 12, 12, 00)) == 29 + + for i in range(1, 31): + assert get_last_day_of_month(datetime(2024, 5, i, 12, 12, 00)) == 31 + + @pytest.mark.parametrize( "gross_price,vat,net_price,vat_price", [ diff --git a/parking_permits/utils.py b/parking_permits/utils.py index 4186779d..f51170cc 100644 --- a/parking_permits/utils.py +++ b/parking_permits/utils.py @@ -3,7 +3,7 @@ import zoneinfo from collections import OrderedDict from collections.abc import Callable -from datetime import datetime +from datetime import datetime, timedelta from decimal import ROUND_UP, Decimal from itertools import chain from typing import Any, Iterable, Iterator, Optional, Union @@ -129,15 +129,30 @@ def get_end_time(start_time, diff_months): return normalize_end_time(end_time) -def increment_end_time(end_time, months=1): +def get_last_day_of_month(date: datetime): + next_month = date.replace(day=28) + timedelta(days=4) + return (next_month - timedelta(days=next_month.day)).day + + +def increment_end_time(start_time, end_time, months=1): """Increment the end time based on the current value (rather than start time). + start_time is used to calculate the original end day, which is used when setting + permit end_time's day after the month has been incremented. Example: 1st Jan 23:59 -> 1st Feb 23:59. Should account for DST changes. """ + original_end_day = (start_time.date() + relativedelta(days=30)).day end_time = end_time.astimezone(tz.get_default_timezone()) + end_time += relativedelta(months=months) + try: + # try to set end day to be same as originally + end_time = end_time.replace(day=original_end_day) + except ValueError: + # end day not in month -> set to last day of the month + end_time = end_time.replace(day=get_last_day_of_month(end_time)) return normalize_end_time(end_time)