Merge branch 'bug/visit_number_not_triggered_properly' into 'master'

Issue #292 and related to issue #195 . Considered update and create events as…

Closes #292

See merge request NCER-PD/scheduling-system!201

smash/web/models/visit.py

@@ -84,17 +84,28 @@ class Visit(models.Model):
             )
 
 @receiver(post_save, sender=Visit)
-def check_visit_number(sender, instance, **kwargs):
+def check_visit_number(sender, instance, created, **kwargs):
     # no other solution to ensure the visit_number is in cronological order than to sort the whole list if there are future visits
     visit = instance
     if visit.subject is not None: #not sure if select_for_update has an effect, the tests work as well without it
-        visits_before = Visit.objects.select_for_update().filter(subject=visit.subject).filter(datetime_begin__lt=visit.datetime_begin).count()
-        # we need to sort the future visits respect to this one, if any
-        visits = Visit.objects.select_for_update().filter(subject=visit.subject).filter(datetime_begin__gte=visit.datetime_begin).order_by('datetime_begin','datetime_end')
-        with transaction.atomic(): #not sure if it has an effect, the tests work as well without it
-            for i, v in enumerate(visits):
-                if v.visit_number != (visits_before + i + 1):
-                    visit_number = (visits_before + i + 1)
-                    Visit.objects.filter(id=v.id).update(visit_number=visit_number) # does not rise post_save, we avoid recursion
-                    if v.id == visit.id:
-                        visit.visit_number = v.visit_number
+        #new visit, sort only future visit respect to the new one
+        if created:
+            visits_before = Visit.objects.select_for_update().filter(subject=visit.subject).filter(datetime_begin__lt=visit.datetime_begin).count()
+            # we need to sort the future visits respect to the new one, if any
+            visits = Visit.objects.select_for_update().filter(subject=visit.subject).filter(datetime_begin__gte=visit.datetime_begin).order_by('datetime_begin','datetime_end')
+            with transaction.atomic(): #not sure if it has an effect, the tests work as well without it
+                for i, v in enumerate(visits):
+                    expected_visit_number = (visits_before + i + 1)
+                    if v.visit_number != expected_visit_number:
+                        Visit.objects.filter(id=v.id).update(visit_number=expected_visit_number) # does not rise post_save, we avoid recursion
+                        if v.id == visit.id: #if the iteration visit is the same that the instance that produced the signal call
+                            #this ensures that the upper saved object is also updated, otherwise, refresh_from_db should be called
+                            visit.visit_number = v.visit_number 
+        else:
+            #if visits are modified, then, check everything
+            visits = Visit.objects.select_for_update().filter(subject=visit.subject).order_by('datetime_begin','datetime_end')
+            with transaction.atomic():
+                for i, v in enumerate(visits):
+                    expected_visit_number = (i+1)
+                    if v.visit_number != expected_visit_number: #update only those with wrong numbers
+                        Visit.objects.filter(id=v.id).update(visit_number=expected_visit_number)

smash/web/tests/models/test_visit.py

@@ -51,6 +51,53 @@ class VisitModelTests(TestCase):
         self.assertEquals(2, visit1.visit_number)
         self.assertEquals(3, visit2.visit_number)
 
+    def test_visit_numbers(self):
+        subject = create_study_subject()
+        Visit.objects.filter(subject=subject).all().delete()
+        visit1 = create_visit(subject=subject, 
+                              datetime_begin = get_today_midnight_date() - datetime.timedelta(days=15),
+                              datetime_end   = get_today_midnight_date() - datetime.timedelta(days=14)) #post save will be raised
+        visit2 = create_visit(subject=subject,
+                              datetime_begin = get_today_midnight_date(),
+                              datetime_end   = get_today_midnight_date() + datetime.timedelta(days=2))  #post save will be raised
+        visit3 = create_visit(subject=subject,
+                              datetime_begin = get_today_midnight_date() + datetime.timedelta(days=5),
+                              datetime_end   = get_today_midnight_date() + datetime.timedelta(days=10)) #post save will be raised
+
+        # order should be [V1, V2, V3]
+        visit1.refresh_from_db()
+        visit2.refresh_from_db()
+        visit3.refresh_from_db()
+        self.assertEquals(1, visit1.visit_number)
+        self.assertEquals(2, visit2.visit_number)
+        self.assertEquals(3, visit3.visit_number)
+
+        #NOW, move V3 between V1 and V2
+        visit3.datetime_begin = get_today_midnight_date() - datetime.timedelta(days=11)
+        visit3.datetime_end   = get_today_midnight_date() - datetime.timedelta(days=10)
+        visit3.save() #post save will be raised and V1, V2 and V3 should change their visits numbers
+
+        #order now should be [V1, V3, V2]
+        visit1.refresh_from_db()
+        visit2.refresh_from_db()
+        visit3.refresh_from_db()
+        self.assertEquals(1, visit1.visit_number)
+        self.assertEquals(3, visit2.visit_number) #<= change
+        self.assertEquals(2, visit3.visit_number) #<= change
+
+        #NOW, move V1 after V2 and V3
+        visit1.datetime_begin = get_today_midnight_date() + datetime.timedelta(days=110)
+        visit1.datetime_end   = get_today_midnight_date() + datetime.timedelta(days=111)
+        visit1.save() #post save will be raised and V1, V2 and V3 should change their visits numbers
+
+        #order now should be [V3, V2, V1]
+        visit1.refresh_from_db()
+        visit2.refresh_from_db()
+        visit3.refresh_from_db()
+        self.assertEquals(3, visit1.visit_number) #<= change        
+        self.assertEquals(2, visit2.visit_number) #<= change
+        self.assertEquals(1, visit3.visit_number) #<= change        
+
     def test_mark_as_finished(self):
         subject = create_study_subject()
         visit = create_visit(subject)