From b306e4cce9f123153920feda3fa950b03f0edc36 Mon Sep 17 00:00:00 2001
From: Tilmann Becker <tilmann.becker+github@gmail.com>
Date: Mon, 14 Aug 2017 15:23:14 +0200
Subject: [PATCH] prevent inherited changes for joined pickups (#346)

* prevent inherited date changes for joined pickups

Closes #596

* prevent inherited changes if users joined a pickup
---
 foodsaving/stores/models.py                   | 37 ++++++++++++-------
 .../stores/tests/test_pickupdates_api.py      | 25 +++++++++++++
 2 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/foodsaving/stores/models.py b/foodsaving/stores/models.py
index 45f0076e1..e82632286 100644
--- a/foodsaving/stores/models.py
+++ b/foodsaving/stores/models.py
@@ -74,32 +74,43 @@ def get_dates_for_rule(self, start_date):
         return [tz.localize(d) for d in dates]
 
     def update_pickup_dates(self, start=timezone.now):
-        # shifting period start time into the future avoids pickup dates which are only valid for a short time
+        """
+        synchronizes the pickup dates with the series
+
+        changes to the series fields are also made to the pickup dates, except for
+        - the field on the pickup date has been modified
+        - users have joined the pickup date
+        """
+
+        # shift start time slightly into future to avoid pickup dates which are only valid for very short time
         start_date = start() + relativedelta(minutes=5)
+
         for pickup, new_date in zip_longest(
             self.pickup_dates.filter(date__gte=start_date),
             self.get_dates_for_rule(start_date=start_date)
         ):
             if not pickup:
-                pickup = PickupDate.objects.create(
+                # does not yet exist
+                PickupDate.objects.create(
                     date=new_date,
                     max_collectors=self.max_collectors,
                     series=self,
                     store=self.store,
                     description=self.description
                 )
-            if not new_date:
-                # only delete pickup dates when they are empty
-                if pickup.collectors.count() <= 0:
+            elif pickup.collectors.count() < 1:
+                # only modify pickups when nobody has joined
+                if not new_date:
+                    # series changed and now this pickup should not exist anymore
                     pickup.delete()
-                    continue
-            if new_date and not pickup.is_date_changed:
-                pickup.date = new_date
-            if not pickup.is_max_collectors_changed:
-                pickup.max_collectors = self.max_collectors
-            if not pickup.is_description_changed:
-                pickup.description = self.description
-            pickup.save()
+                else:
+                    if not pickup.is_date_changed:
+                        pickup.date = new_date
+                    if not pickup.is_max_collectors_changed:
+                        pickup.max_collectors = self.max_collectors
+                    if not pickup.is_description_changed:
+                        pickup.description = self.description
+                    pickup.save()
 
     def __str__(self):
         return '{} - {}'.format(self.date, self.store)
diff --git a/foodsaving/stores/tests/test_pickupdates_api.py b/foodsaving/stores/tests/test_pickupdates_api.py
index 75e5ee14a..cd3bd74f8 100644
--- a/foodsaving/stores/tests/test_pickupdates_api.py
+++ b/foodsaving/stores/tests/test_pickupdates_api.py
@@ -383,6 +383,31 @@ def test_dont_mark_as_changed_if_data_is_equal(self):
         self.assertFalse(pickup_under_test.is_max_collectors_changed)
         self.assertFalse(pickup_under_test.is_date_changed)
 
+    def test_keep_date_if_pickup_has_collectors(self):
+        """
+        https://github.com/yunity/foodsaving-frontend/issues/596
+        It's unexpected if the date changes automatically when people have joined
+        """
+        self.client.force_login(user=self.member)
+        pickup_under_test = self.series.pickup_dates.first()
+        url = '/api/pickup-dates/{}/add/'.format(pickup_under_test.id)
+
+        # join pickup
+        response = self.client.post(url)
+        self.assertEqual(response.status_code, status.HTTP_200_OK, response.data)
+        original_date = pickup_under_test.date
+
+        # change series date
+        url = '/api/pickup-date-series/{}/'.format(self.series.id)
+        response = self.client.patch(url, {'start_date': self.series.start_date + relativedelta(hours=1)})
+        self.assertEqual(response.status_code, status.HTTP_200_OK, response.data)
+
+        # check if time of pickup is the same
+        url = '/api/pickup-dates/{}/'.format(pickup_under_test.id)
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, status.HTTP_200_OK, response.data)
+        self.assertEqual(parse(response.data['date']), original_date, "time shouldn't change!")
+
 
 class TestPickupDateSeriesAPIAuth(APITestCase):
     """ Testing actions that are forbidden """