From 16ebacdac42d28d3b2f6cfdd05c2f582b9970d73 Mon Sep 17 00:00:00 2001 From: RomainFayolle Date: Wed, 5 Jul 2023 15:59:19 -0400 Subject: [PATCH 1/8] refactor force refund to handle multiple refund policies --- retirement/models.py | 49 ++++++++++++------- retirement/tests/tests_viewset_Retreat.py | 2 +- retirement/tests/tests_viewset_RetreatDate.py | 12 ++--- retirement/views.py | 18 +++---- 4 files changed, 47 insertions(+), 34 deletions(-) diff --git a/retirement/models.py b/retirement/models.py index 28f25f8b..49284361 100644 --- a/retirement/models.py +++ b/retirement/models.py @@ -999,7 +999,7 @@ def get_participants(self): participants.add(reservation.user) return list(participants) - def process_impacted_users(self, reason, reason_message, force_refund): + def process_impacted_users(self, reason, reason_message, refund_policy): """ Notify and potentially refund user for a reason happening on retreat: Retreat cancelled, deleted ... @@ -1010,7 +1010,7 @@ def process_impacted_users(self, reason, reason_message, force_refund): # retrieve active users before we cancel the reservation users = self.get_participants() from .services import send_updated_retreat_email - self.cancel_participants_reservation(force_refund) + self.cancel_participants_reservation(refund_policy) send_updated_retreat_email( self, users, @@ -1018,11 +1018,10 @@ def process_impacted_users(self, reason, reason_message, force_refund): reason_message, ) - def cancel_participants_reservation(self, force_refund): + def cancel_participants_reservation(self, refund_policy): """ Cancel all participants' reservation - :params force_refund: True if we want to force refund for participants, - otherwise regular refund will be done. + :params refund_policy: Admin policy to apply to refund participants """ active_reservations = self.reservations.filter(is_active=True) refund_data = [] @@ -1033,7 +1032,7 @@ def cancel_participants_reservation(self, force_refund): refund_data.append( reservation.process_refund( Reservation.CANCELATION_REASON_RETREAT_DELETED, - force_refund) + refund_policy) ) # Send emails to each participant having a refund @@ -1042,7 +1041,7 @@ def cancel_participants_reservation(self, force_refund): if data: Reservation.send_refund_confirmation_email(data) - def custom_delete(self, deletion_message=None, force_refund=False): + def custom_delete(self, deletion_message=None, refund_policy=None): """ Deleting a retreat sends an email to all registered participants set the retreat to inactive and hide it from the admin panel. @@ -1051,7 +1050,8 @@ def custom_delete(self, deletion_message=None, force_refund=False): """ self.is_active = False self.hide_from_client_admin_panel = True - self.process_impacted_users('deletion', deletion_message, force_refund) + self.process_impacted_users( + 'deletion', deletion_message, refund_policy) self.save() @@ -1257,8 +1257,14 @@ class Reservation(SafeDeleteModel): def __str__(self): return str(self.user) - def get_refund_value(self, total_refund=False): - if self.order_line is None or self.order_line.is_made_by_admin: + def get_refund_value(self, refund_policy=None): + """ + Refund the user. Admin can specify a refund_policy to fully refund + user, or not refund at all or refund at retreat rate. + """ + if self.order_line is None or \ + self.order_line.is_made_by_admin or \ + refund_policy == 'no_refund': return 0 # First get net pay: total cost @@ -1266,7 +1272,7 @@ def get_refund_value(self, total_refund=False): # Add the tax rate, so we have the real value pay by the user refund_value *= TAX_RATE + 1.0 - if not total_refund: + if refund_policy in ['retreat_rate', None]: # keep only the part that the retreat allow to refund refund_value *= self.retreat.refund_rate / 100 @@ -1279,8 +1285,8 @@ def get_refund_value(self, total_refund=False): return round(refund_value, 2) if refund_value > 0 else 0 - def make_refund(self, refund_reason, total_refund=False): - amount_to_refund = self.get_refund_value(total_refund) + def make_refund(self, refund_reason, refund_policy=None): + amount_to_refund = self.get_refund_value(refund_policy) # paysafe use value without cent amount_to_refund_paysafe = int(round(amount_to_refund * 100)) @@ -1362,7 +1368,7 @@ def send_refund_confirmation_email(email_dict): ) raise - def process_refund(self, cancel_reason, force_refund): + def process_refund(self, cancel_reason, refund_policy): """ User will be refund the retreat's "refund_rate" if we're at least "min_day_refund" days before the event. @@ -1398,12 +1404,17 @@ def process_refund(self, cancel_reason, force_refund): # refundable # # 2 - An admin want to force a refund and the user paid for - # his reservation + # his reservation or force no refund # # In all case, only paid reservation (amount > 0) can be refunded if self.get_refund_value() > 0: - process_refund = (respects_minimum_days and refundable) or \ - force_refund + if refund_policy: + if refund_policy in ['retreat_rate', 'full_rate']: + process_refund = True + else: + process_refund = False + else: + process_refund = respects_minimum_days and refundable else: process_refund = False @@ -1422,7 +1433,9 @@ def process_refund(self, cancel_reason, force_refund): if process_refund: try: refund = self.make_refund( - self.REFUND_REASON[cancel_reason]) + self.REFUND_REASON[cancel_reason], + refund_policy + ) except PaymentAPIError as err: if str(err) == PAYSAFE_EXCEPTION['3406']: raise rest_framework_serializers.ValidationError({ diff --git a/retirement/tests/tests_viewset_Retreat.py b/retirement/tests/tests_viewset_Retreat.py index e2eb5bf8..1c506603 100644 --- a/retirement/tests/tests_viewset_Retreat.py +++ b/retirement/tests/tests_viewset_Retreat.py @@ -1115,7 +1115,7 @@ def test_delete_with_participants(self, mock_email, mock_cancel): 'deletion', deletion_message ) - mock_cancel.assert_called_once_with(False) + mock_cancel.assert_called_once_with(None) self.retreat.refresh_from_db() self.assertFalse(self.retreat.is_active) diff --git a/retirement/tests/tests_viewset_RetreatDate.py b/retirement/tests/tests_viewset_RetreatDate.py index 27383674..83425039 100644 --- a/retirement/tests/tests_viewset_RetreatDate.py +++ b/retirement/tests/tests_viewset_RetreatDate.py @@ -208,7 +208,7 @@ def test_update_no_change_as_admin(self, mock_email, mock_cancel): 'update', reason_message, ) - mock_cancel.assert_called_once_with(False) + mock_cancel.assert_called_once_with(None) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -258,7 +258,7 @@ def test_update_change_after_as_admin(self, mock_email, mock_cancel): 'update', reason_message, ) - mock_cancel.assert_called_once_with(False) + mock_cancel.assert_called_once_with(None) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -308,7 +308,7 @@ def test_update_change_before_as_admin(self, mock_email, mock_cancel): 'update', reason_message, ) - mock_cancel.assert_called_once_with(False) + mock_cancel.assert_called_once_with(None) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -378,7 +378,7 @@ def test_delete_no_change_as_admin(self, mock_email, mock_cancel): 'update', reason_message, ) - mock_cancel.assert_called_once_with(False) + mock_cancel.assert_called_once_with(None) self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) @@ -426,7 +426,7 @@ def test_delete_change_after_as_admin(self, mock_email, mock_cancel): 'update', reason_message, ) - mock_cancel.assert_called_once_with(False) + mock_cancel.assert_called_once_with(None) self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) @@ -474,7 +474,7 @@ def test_delete_change_before_as_admin(self, mock_email, mock_cancel): 'update', reason_message, ) - mock_cancel.assert_called_once_with(False) + mock_cancel.assert_called_once_with(None) self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) diff --git a/retirement/views.py b/retirement/views.py index eb8f648f..5c43da62 100644 --- a/retirement/views.py +++ b/retirement/views.py @@ -217,7 +217,7 @@ def get_queryset(self): def destroy(self, request, *args, **kwargs): instance = self.get_object() deletion_message = request.data.get('deletion_message', None) - force_refund = request.data.get('force_refund', False) + refund_policy = request.data.get('refund_policy', None) if instance.total_reservations > 0 and not deletion_message: error = { 'deletion_message': _("There is at least one participant to " @@ -226,7 +226,7 @@ def destroy(self, request, *args, **kwargs): } return Response(error, status=status.HTTP_400_BAD_REQUEST) if instance.is_active: - instance.custom_delete(deletion_message, force_refund) + instance.custom_delete(deletion_message, refund_policy) elif not instance.hide_from_client_admin_panel: instance.hide_from_client_admin_panel = True instance.save() @@ -632,16 +632,16 @@ def destroy(self, request, *args, **kwargs): instance = self.get_object() user = instance.user - force_refund = False + refund_policy = None if self.request.user.is_staff: - force_refund = request.data.get('force_refund', False) + refund_policy = request.data.get('refund_policy', None) if self.request.user.id != user.id: cancel_reason = Reservation.CANCELATION_REASON_ADMIN_CANCELLED else: cancel_reason = Reservation.CANCELATION_REASON_USER_CANCELLED - refund_data = instance.process_refund(cancel_reason, force_refund) + refund_data = instance.process_refund(cancel_reason, refund_policy) if refund_data: Reservation.send_refund_confirmation_email(refund_data) @@ -777,7 +777,7 @@ class RetreatDateViewSet(viewsets.ModelViewSet): def update(self, request, *args, **kwargs): retreat_date: RetreatDate = self.get_object() reason_message = request.data.get('reason_message', None) - force_refund = request.data.get('force_refund', False) + refund_policy = request.data.get('refund_policy', None) if retreat_date.retreat.total_reservations > 0 and \ not reason_message: error = { @@ -796,13 +796,13 @@ def update(self, request, *args, **kwargs): update(request, *args, **kwargs) retreat.set_automatic_email() # Recalculate retreat auto-email retreat.process_impacted_users( - 'update', reason_message, force_refund) + 'update', reason_message, refund_policy) return response def destroy(self, request, *args, **kwargs): retreat_date: RetreatDate = self.get_object() reason_message = request.data.get('reason_message', None) - force_refund = request.data.get('force_refund', False) + refund_policy = request.data.get('refund_policy', None) if retreat_date.retreat.total_reservations > 0 and \ not reason_message: error = { @@ -827,7 +827,7 @@ def destroy(self, request, *args, **kwargs): request, *args, **kwargs) retreat.set_automatic_email() # Recalculate retreat auto-email retreat.process_impacted_users( - 'update', reason_message, force_refund) + 'update', reason_message, refund_policy) return Response(status=status.HTTP_204_NO_CONTENT) From 6270c42f98949872ab4e38e29aedbc1914490819 Mon Sep 17 00:00:00 2001 From: RomainFayolle Date: Mon, 17 Jul 2023 08:58:12 -0400 Subject: [PATCH 2/8] update email dict for different rate --- retirement/models.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/retirement/models.py b/retirement/models.py index 49284361..b06acf44 100644 --- a/retirement/models.py +++ b/retirement/models.py @@ -1317,10 +1317,19 @@ def send_refund_confirmation_email(email_dict): user = email_dict['user'] total_amount = email_dict['total_amount'] amount_tax = email_dict['amount_tax'] + refund_policy = email_dict['refund_policy'] # Here the price takes the applied coupon into account, if # applicable. + if refund_policy == 'full_rate': + price = amount + elif refund_policy == 'retreat_rate': + price = (amount * retreat.refund_rate) / 100 + elif refund_policy == 'no_refund': + price = 0 + else: + raise Exception(f'Refund policy {refund_policy} not handled') old_retreat = { - 'price': (amount * retreat.refund_rate) / 100, + 'price': price, 'name': "{0}: {1}".format( _("Retreat"), retreat.name @@ -1499,6 +1508,7 @@ def process_refund(self, cancel_reason, refund_policy): 'user': user, 'total_amount': refund.amount, 'amount_tax': round(refund.amount * TAX_RATE, 2), + 'refund_policy': refund_policy, } return email_data From a8fb6e99a7ed4a630de10408c952b04ac987ccdd Mon Sep 17 00:00:00 2001 From: RomainFayolle Date: Tue, 18 Jul 2023 11:08:28 -0400 Subject: [PATCH 3/8] update email dict for different rate --- retirement/models.py | 17 ++++++++++------- retirement/tests/tests_viewset_Reservation.py | 1 + store/tests/tests_viewset_Order.py | 1 + 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/retirement/models.py b/retirement/models.py index b06acf44..4c6b3d7b 100644 --- a/retirement/models.py +++ b/retirement/models.py @@ -1321,18 +1321,21 @@ def send_refund_confirmation_email(email_dict): # Here the price takes the applied coupon into account, if # applicable. if refund_policy == 'full_rate': - price = amount - elif refund_policy == 'retreat_rate': - price = (amount * retreat.refund_rate) / 100 + refund_rate = 100 + elif refund_policy in ['retreat_rate', None]: + refund_rate = retreat.refund_rate elif refund_policy == 'no_refund': - price = 0 + refund_rate = 0 else: raise Exception(f'Refund policy {refund_policy} not handled') old_retreat = { - 'price': price, - 'name': "{0}: {1}".format( + 'price': amount, + 'name': "{0}: {1} - {2}$ CAD | {3}% {4}".format( _("Retreat"), - retreat.name + retreat.name, + retreat.price, + refund_rate, + _("refund rate") ) } diff --git a/retirement/tests/tests_viewset_Reservation.py b/retirement/tests/tests_viewset_Reservation.py index 639b0cc8..123b57e0 100644 --- a/retirement/tests/tests_viewset_Reservation.py +++ b/retirement/tests/tests_viewset_Reservation.py @@ -1332,6 +1332,7 @@ def test_delete_scheduler_working(self): 'retreat:reservation-detail', kwargs={'pk': self.reservation_admin.id}, ), + data={"refund_policy": "retreat_rate"}, ) self.assertEqual( diff --git a/store/tests/tests_viewset_Order.py b/store/tests/tests_viewset_Order.py index 3e1b1959..0ea227aa 100644 --- a/store/tests/tests_viewset_Order.py +++ b/store/tests/tests_viewset_Order.py @@ -4152,6 +4152,7 @@ def test_refund(self): 'retreat:reservation-detail', kwargs={'pk': reservation.id}, ), + data={"refund_policy": "retreat_rate"}, ) self.assertEqual( From 0715e79d67d44365cffd32661a066bd82358538b Mon Sep 17 00:00:00 2001 From: RomainFayolle Date: Tue, 18 Jul 2023 14:58:50 -0400 Subject: [PATCH 4/8] display less information avoid understanding errors with options, since the template is used also by resevration update --- retirement/models.py | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/retirement/models.py b/retirement/models.py index 4c6b3d7b..21930d0a 100644 --- a/retirement/models.py +++ b/retirement/models.py @@ -1317,25 +1317,13 @@ def send_refund_confirmation_email(email_dict): user = email_dict['user'] total_amount = email_dict['total_amount'] amount_tax = email_dict['amount_tax'] - refund_policy = email_dict['refund_policy'] # Here the price takes the applied coupon into account, if # applicable. - if refund_policy == 'full_rate': - refund_rate = 100 - elif refund_policy in ['retreat_rate', None]: - refund_rate = retreat.refund_rate - elif refund_policy == 'no_refund': - refund_rate = 0 - else: - raise Exception(f'Refund policy {refund_policy} not handled') old_retreat = { 'price': amount, - 'name': "{0}: {1} - {2}$ CAD | {3}% {4}".format( + 'name': "{0}: {1}".format( _("Retreat"), retreat.name, - retreat.price, - refund_rate, - _("refund rate") ) } From bccf48cbf163c8389e350b28155c0d16002467a3 Mon Sep 17 00:00:00 2001 From: RignonNoel Date: Thu, 3 Aug 2023 10:12:07 -0400 Subject: [PATCH 5/8] add geolocation_link to workplace model --- .../migrations/0026_auto_20230803_0956.py | 23 +++++++++++++++++++ workplace/models.py | 6 +++++ workplace/tests/tests_viewset_TimeSlot.py | 11 +++++++++ workplace/tests/tests_viewset_Workplace.py | 5 ++++ 4 files changed, 45 insertions(+) create mode 100644 workplace/migrations/0026_auto_20230803_0956.py diff --git a/workplace/migrations/0026_auto_20230803_0956.py b/workplace/migrations/0026_auto_20230803_0956.py new file mode 100644 index 00000000..d530b28f --- /dev/null +++ b/workplace/migrations/0026_auto_20230803_0956.py @@ -0,0 +1,23 @@ +# Generated by Django 3.2.8 on 2023-08-03 13:56 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('workplace', '0025_auto_20230203_1134'), + ] + + operations = [ + migrations.AddField( + model_name='historicalworkplace', + name='geolocation_link', + field=models.TextField(blank=True, null=True, verbose_name='Geolocation link'), + ), + migrations.AddField( + model_name='workplace', + name='geolocation_link', + field=models.TextField(blank=True, null=True, verbose_name='Geolocation link'), + ), + ] diff --git a/workplace/models.py b/workplace/models.py index 8989091e..17e51bb1 100644 --- a/workplace/models.py +++ b/workplace/models.py @@ -30,6 +30,12 @@ class Meta: max_length=1000, ) + geolocation_link = models.TextField( + verbose_name=_("Geolocation link"), + null=True, + blank=True, + ) + seats = models.IntegerField( verbose_name=_("Seats"), ) diff --git a/workplace/tests/tests_viewset_TimeSlot.py b/workplace/tests/tests_viewset_TimeSlot.py index e4fc1459..9864d685 100644 --- a/workplace/tests/tests_viewset_TimeSlot.py +++ b/workplace/tests/tests_viewset_TimeSlot.py @@ -507,6 +507,7 @@ def test_update(self): "latitude": None, "longitude": None, "name": "Blitz", + "geolocation_link": None, "pictures": [], "postal_code": "123 456", "seats": 40, @@ -652,6 +653,7 @@ def test_update_safe(self, send): "latitude": None, "longitude": None, "name": "Blitz", + "geolocation_link": None, "pictures": [], "postal_code": "123 456", "seats": 40, @@ -816,6 +818,7 @@ def test_update_timeslot_with_registered_users_partial(self): "latitude": None, "longitude": None, "name": "Blitz", + "geolocation_link": None, "pictures": [], "postal_code": "123 456", "seats": 40, @@ -908,6 +911,7 @@ def test_update_timeslot_failed_emails(self, send): "latitude": None, "longitude": None, "name": "Blitz", + "geolocation_link": None, "pictures": [], "postal_code": "123 456", "seats": 40, @@ -1111,6 +1115,7 @@ def test_list(self): "latitude": None, "longitude": None, "name": "Blitz2", + "geolocation_link": None, "pictures": [], "postal_code": "123 456", "seats": 40, @@ -1170,6 +1175,7 @@ def test_list_inactive(self): "latitude": None, "longitude": None, "name": "Blitz", + "geolocation_link": None, "pictures": [], "postal_code": "123 456", "seats": 40, @@ -1202,6 +1208,7 @@ def test_list_inactive(self): "latitude": None, "longitude": None, "name": "Blitz2", + "geolocation_link": None, "pictures": [], "postal_code": "123 456", "seats": 40, @@ -1271,6 +1278,7 @@ def test_list_filter_by_workplace(self): "latitude": None, "longitude": None, "name": "Blitz", + "geolocation_link": None, "pictures": [], "postal_code": "123 456", "seats": 40, @@ -1388,6 +1396,7 @@ def test_read(self): "latitude": None, "longitude": None, "name": "Blitz2", + "geolocation_link": None, "pictures": [], "postal_code": "123 456", "seats": 40, @@ -1444,6 +1453,7 @@ def test_read_reserved(self): "latitude": None, "longitude": None, "name": "Blitz2", + "geolocation_link": None, "pictures": [], "postal_code": "123 456", "seats": 40, @@ -1517,6 +1527,7 @@ def test_read_inactive(self): "latitude": None, "longitude": None, "name": "Blitz", + "geolocation_link": None, "pictures": [], "postal_code": "123 456", "seats": 40, diff --git a/workplace/tests/tests_viewset_Workplace.py b/workplace/tests/tests_viewset_Workplace.py index 11c4b849..2495258e 100644 --- a/workplace/tests/tests_viewset_Workplace.py +++ b/workplace/tests/tests_viewset_Workplace.py @@ -77,6 +77,7 @@ def test_create(self): 'latitude': None, 'longitude': None, 'name': 'random_workplace', + 'geolocation_link': None, 'pictures': [], 'seats': 40, 'timezone': "America/Montreal", @@ -245,6 +246,7 @@ def test_update(self): 'address_line1': 'new_address', 'city': 'new_city', 'country': 'Random_Country', + 'geolocation_link': 'https://goo.gl/maps/YmWaRRvShFQ2MFMs6', 'postal_code': 'NEW_CIT', 'state_province': 'Random_State', 'timezone': "America/Montreal", @@ -271,6 +273,7 @@ def test_update(self): 'postal_code': 'NEW_CIT', 'state_province': 'Random_State', 'name': 'new_workplace', + 'geolocation_link': 'https://goo.gl/maps/YmWaRRvShFQ2MFMs6', 'pictures': [], 'seats': 200, 'timezone': 'America/Montreal', @@ -327,6 +330,7 @@ def test_list(self): 'postal_code': 'RAN_DOM', 'state_province': 'Random_State', 'name': 'Blitz', + 'geolocation_link': None, 'pictures': [], 'seats': 40, 'timezone': 'America/Montreal', @@ -364,6 +368,7 @@ def test_read(self): 'postal_code': 'RAN_DOM', 'state_province': 'Random_State', 'name': 'Blitz', + 'geolocation_link': None, 'pictures': [], 'seats': 40, 'place_name': '', From 92b3d506e7581e3e73630f013b442cbfa2182c85 Mon Sep 17 00:00:00 2001 From: RomainFayolle Date: Thu, 17 Aug 2023 10:09:21 -0400 Subject: [PATCH 6/8] implement new field is_accessible for workplace --- workplace/admin.py | 2 ++ .../migrations/0027_auto_20230817_0923.py | 23 +++++++++++++++++++ workplace/models.py | 5 ++++ workplace/views.py | 7 ++++++ 4 files changed, 37 insertions(+) create mode 100644 workplace/migrations/0027_auto_20230817_0923.py diff --git a/workplace/admin.py b/workplace/admin.py index e2e9de78..a7d13c6a 100644 --- a/workplace/admin.py +++ b/workplace/admin.py @@ -24,11 +24,13 @@ class WorkplaceAdmin(SimpleHistoryAdmin, SafeDeleteAdmin, TranslationAdmin, list_display = ( 'name', 'seats', + 'is_accessible', highlight_deleted, ) + SafeDeleteAdmin.list_display list_filter = ( 'name', 'seats', + 'is_accessible', ) + SafeDeleteAdmin.list_filter actions = ['undelete_selected', 'export_admin_action'] diff --git a/workplace/migrations/0027_auto_20230817_0923.py b/workplace/migrations/0027_auto_20230817_0923.py new file mode 100644 index 00000000..f086a201 --- /dev/null +++ b/workplace/migrations/0027_auto_20230817_0923.py @@ -0,0 +1,23 @@ +# Generated by Django 3.2.8 on 2023-08-17 13:23 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('workplace', '0026_auto_20230803_0956'), + ] + + operations = [ + migrations.AddField( + model_name='historicalworkplace', + name='is_accessible', + field=models.BooleanField(default=True, verbose_name='Is accessible'), + ), + migrations.AddField( + model_name='workplace', + name='is_accessible', + field=models.BooleanField(default=True, verbose_name='Is accessible'), + ), + ] diff --git a/workplace/models.py b/workplace/models.py index 17e51bb1..8c7c910f 100644 --- a/workplace/models.py +++ b/workplace/models.py @@ -47,6 +47,11 @@ class Meta: related_name='workplaces', ) + is_accessible = models.BooleanField( + default=True, + verbose_name=_("Is accessible"), + ) + # History is registered in translation.py # history = HistoricalRecords() diff --git a/workplace/views.py b/workplace/views.py index 85775494..e370819a 100644 --- a/workplace/views.py +++ b/workplace/views.py @@ -50,6 +50,13 @@ class WorkplaceViewSet(ExportMixin, viewsets.ModelViewSet): export_resource = WorkplaceResource() + def get_queryset(self): + if self.request.user.is_staff: + queryset = Workplace.objects.all() + else: + queryset = Workplace.objects.filter(is_accessible=True) + return queryset + class PictureViewSet(viewsets.ModelViewSet): """ From 48922d25df4c550464c1718e96800c8585f1a7e3 Mon Sep 17 00:00:00 2001 From: RomainFayolle Date: Thu, 17 Aug 2023 10:11:54 -0400 Subject: [PATCH 7/8] add and update tests --- workplace/tests/tests_model_Workplace.py | 1 + workplace/tests/tests_viewset_TimeSlot.py | 33 ++++++--- workplace/tests/tests_viewset_Workplace.py | 83 ++++++++++++++++++++-- 3 files changed, 101 insertions(+), 16 deletions(-) diff --git a/workplace/tests/tests_model_Workplace.py b/workplace/tests/tests_model_Workplace.py index f610221e..9a793743 100644 --- a/workplace/tests/tests_model_Workplace.py +++ b/workplace/tests/tests_model_Workplace.py @@ -17,6 +17,7 @@ def test_create(self): postal_code="123 456", state_province="Random state", country="Random country", + is_accessible=True, ) self.assertEqual(workplace.__str__(), "random_workplace") diff --git a/workplace/tests/tests_viewset_TimeSlot.py b/workplace/tests/tests_viewset_TimeSlot.py index 9864d685..e0cbf1fb 100644 --- a/workplace/tests/tests_viewset_TimeSlot.py +++ b/workplace/tests/tests_viewset_TimeSlot.py @@ -515,7 +515,8 @@ def test_update(self): "timezone": None, 'place_name': '', "volunteers": [], - "url": f"http://testserver/workplaces/{self.workplace.id}" + "url": f"http://testserver/workplaces/{self.workplace.id}", + 'is_accessible': True, } } @@ -661,7 +662,8 @@ def test_update_safe(self, send): "timezone": None, 'place_name': '', "volunteers": [], - "url": f"http://testserver/workplaces/{self.workplace.id}" + "url": f"http://testserver/workplaces/{self.workplace.id}", + 'is_accessible': True, } } @@ -826,7 +828,8 @@ def test_update_timeslot_with_registered_users_partial(self): "timezone": None, 'place_name': '', "volunteers": [], - "url": f"http://testserver/workplaces/{self.workplace.id}" + "url": f"http://testserver/workplaces/{self.workplace.id}", + 'is_accessible': True, } } @@ -919,7 +922,8 @@ def test_update_timeslot_failed_emails(self, send): "timezone": None, 'place_name': '', "volunteers": [], - "url": f"http://testserver/workplaces/{self.workplace.id}" + "url": f"http://testserver/workplaces/{self.workplace.id}", + 'is_accessible': True, } } @@ -1123,7 +1127,8 @@ def test_list(self): "timezone": None, "place_name": '', "volunteers": [], - "url": f"http://testserver/workplaces/{self.workplace2.id}" + "url": f"http://testserver/workplaces/{self.workplace2.id}", + 'is_accessible': True, } }] } @@ -1183,7 +1188,8 @@ def test_list_inactive(self): "timezone": None, "place_name": '', "volunteers": [], - "url": f"http://testserver/workplaces/{self.workplace.id}" + "url": f"http://testserver/workplaces/{self.workplace.id}", + 'is_accessible': True, } }, { 'id': self.time_slot_active.id, @@ -1216,7 +1222,8 @@ def test_list_inactive(self): "timezone": None, "place_name": '', "volunteers": [], - "url": f"http://testserver/workplaces/{self.workplace2.id}" + "url": f"http://testserver/workplaces/{self.workplace2.id}", + 'is_accessible': True, } }] } @@ -1286,7 +1293,8 @@ def test_list_filter_by_workplace(self): "timezone": None, "place_name": '', "volunteers": [], - "url": f"http://testserver/workplaces/{self.workplace.id}" + "url": f"http://testserver/workplaces/{self.workplace.id}", + 'is_accessible': True, } }] } @@ -1404,7 +1412,8 @@ def test_read(self): "timezone": None, "place_name": '', "volunteers": [], - "url": f"http://testserver/workplaces/{self.workplace2.id}" + "url": f"http://testserver/workplaces/{self.workplace2.id}", + 'is_accessible': True, } } @@ -1461,7 +1470,8 @@ def test_read_reserved(self): "timezone": None, "place_name": '', "volunteers": [], - "url": f"http://testserver/workplaces/{self.workplace2.id}" + "url": f"http://testserver/workplaces/{self.workplace2.id}", + 'is_accessible': True, } } @@ -1535,7 +1545,8 @@ def test_read_inactive(self): "timezone": None, "place_name": '', "volunteers": [], - "url": f"http://testserver/workplaces/{self.workplace.id}" + "url": f"http://testserver/workplaces/{self.workplace.id}", + 'is_accessible': True, } } diff --git a/workplace/tests/tests_viewset_Workplace.py b/workplace/tests/tests_viewset_Workplace.py index 2495258e..0e68ebc8 100644 --- a/workplace/tests/tests_viewset_Workplace.py +++ b/workplace/tests/tests_viewset_Workplace.py @@ -53,6 +53,7 @@ def test_create(self): 'state_province': 'Random_State', 'timezone': "America/Montreal", 'volunteers': [f"http://testserver/users/{self.user.id}"], + 'is_accessible': False, } response = self.client.post( @@ -85,6 +86,7 @@ def test_create(self): 'volunteers': [ f'http://testserver/users/{self.user.id}' ], + 'is_accessible': False, } del response_content['id'] @@ -250,6 +252,7 @@ def test_update(self): 'postal_code': 'NEW_CIT', 'state_province': 'Random_State', 'timezone': "America/Montreal", + 'is_accessible': False, } response = self.client.put( @@ -279,7 +282,8 @@ def test_update(self): 'timezone': 'America/Montreal', 'place_name': '', 'volunteers': [], - 'url': f'http://testserver/workplaces/{self.workplace.id}' + 'url': f'http://testserver/workplaces/{self.workplace.id}', + 'is_accessible': False, } self.assertEqual( @@ -304,11 +308,25 @@ def test_delete(self): self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) - def test_list(self): + def test_list_unauthenticated(self): """ - Ensure we can list workplaces as an unauthenticated user. + Ensure we can list workplaces as an unauthenticated user. Only seeing + accessible worplaces """ + Workplace.objects.create( + name="Inaccessible", + seats=40, + details="short_description", + address_line1="random_address_1", + postal_code="RAN_DOM", + city='random_city', + state_province="Random_State", + country="Random_Country", + timezone="America/Montreal", + is_accessible=False, + ) + response = self.client.get( reverse('workplace-list'), format='json', @@ -336,7 +354,8 @@ def test_list(self): 'timezone': 'America/Montreal', 'place_name': '', 'volunteers': [], - 'url': f'http://testserver/workplaces/{self.workplace.id}' + 'url': f'http://testserver/workplaces/{self.workplace.id}', + 'is_accessible': True, }] } @@ -344,6 +363,59 @@ def test_list(self): self.assertEqual(response.status_code, status.HTTP_200_OK) + def test_list_admin(self): + """ + Ensure we can list workplaces as an admin user. + """ + self.client.force_authenticate(user=self.admin) + Workplace.objects.create( + name="Inaccessible", + seats=40, + details="short_description", + address_line1="random_address_1", + postal_code="RAN_DOM", + city='random_city', + state_province="Random_State", + country="Random_Country", + timezone="America/Montreal", + is_accessible=False, + ) + + response = self.client.get( + reverse('workplace-list'), + format='json', + ) + + self.assertEqual(json.loads(response.content)['count'], 2) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_list_user(self): + """ + Ensure we can list workplaces as user, only seeing accessible + """ + + Workplace.objects.create( + name="Inaccessible", + seats=40, + details="short_description", + address_line1="random_address_1", + postal_code="RAN_DOM", + city='random_city', + state_province="Random_State", + country="Random_Country", + timezone="America/Montreal", + is_accessible=False, + ) + self.client.force_authenticate(user=self.user) + response = self.client.get( + reverse('workplace-list'), + format='json', + ) + content = json.loads(response.content) + self.assertEqual(content['count'], 1) + self.assertEqual(response.status_code, status.HTTP_200_OK) + def test_read(self): """ Ensure we can read a workplace as an unauthenticated user. @@ -374,7 +446,8 @@ def test_read(self): 'place_name': '', 'timezone': 'America/Montreal', 'volunteers': [], - 'url': f'http://testserver/workplaces/{self.workplace.id}' + 'url': f'http://testserver/workplaces/{self.workplace.id}', + 'is_accessible': True, } self.assertEqual(json.loads(response.content), content) From f6eb480a9c6779c63af3471abc18d0beae37bbe9 Mon Sep 17 00:00:00 2001 From: RomainFayolle Date: Thu, 17 Aug 2023 10:12:10 -0400 Subject: [PATCH 8/8] remove unused import --- workplace/serializers.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/workplace/serializers.py b/workplace/serializers.py index 1d20a7ce..c91ef9dd 100644 --- a/workplace/serializers.py +++ b/workplace/serializers.py @@ -3,13 +3,11 @@ from datetime import datetime -from dateutil.parser import parse from dateutil.rrule import rrule, DAILY import pytz -from rest_framework import serializers, status -from rest_framework.reverse import reverse +from rest_framework import serializers from rest_framework.validators import UniqueValidator from django.conf import settings