Merge pull request #1085 from eciis/fix-duplicated-child-inst

Fix duplicated institution children

backend/handlers/institution_children_request_handler.py

@@ -30,11 +30,9 @@ def put(self, user, request_key):
                               'User is not allowed to accept link between institutions',
                               request.institution_requested_key.urlsafe())
         request.change_status('accepted')
-        request.put()
 
         institution_children = request.institution_requested_key.get()
-        institution_children.parent_institution = request.institution_key
-        institution_children.put()
+        institution_children.set_parent(request.institution_key)
 
         request.send_response_notification(user.current_institution, user.key, 'ACCEPT')
         request.send_response_email('ACCEPT')
@@ -52,7 +50,11 @@ def delete(self, user, request_key):
                               'User is not allowed to reject link between institutions',
                               request.institution_requested_key.urlsafe())
         request.change_status('rejected')
-        request.put()
+
+        institution_children = request.institution_requested_key.get()
+
+        if institution_children.parent_institution == request.institution_key:
+            institution_children.set_parent(None)
 
         request.send_response_notification(user.current_institution, user.key, 'REJECT')
         request.send_response_email('REJECT')

backend/handlers/institution_parent_request_handler.py

@@ -30,14 +30,12 @@ def put(self, user, request_key):
                               'User is not allowed to accept link between institutions',
                               request.institution_requested_key.urlsafe())
         request.change_status('accepted')
-        request.put()
 
         parent_institution = request.institution_requested_key.get()
-        parent_institution.children_institutions.append(request.institution_key)
-        parent_institution.put()
+        parent_institution.add_child(request.institution_key)
 
         institution_children = request.institution_key.get()
-        
+
         request.send_response_notification(user.current_institution, user.key, 'ACCEPT')
         request.send_response_email('ACCEPT')
 
@@ -54,7 +52,9 @@ def delete(self, user, request_key):
                               'User is not allowed to reject link between institutions',
                               request.institution_requested_key.urlsafe())
         request.change_status('rejected')
-        request.put()
+
+        parent_institution = request.institution_requested_key.get()
+        parent_institution.remove_child(request.institution_key)
 
         request.send_response_notification(user.current_institution, user.key, 'REJECT')
         request.send_response_email('REJECT')

backend/models/institution.py

@@ -132,6 +132,11 @@ def add_child(self, child_key):
             self.children_institutions.append(child_key)
             self.put()
 
+    def remove_child(self, child_key):
+        if child_key in self.children_institutions:
+            self.children_institutions.remove(child_key)
+            self.put()
+
     def set_parent(self, parent_key):
         """Set a new parent."""
         self.parent_institution = parent_key

backend/test/institution_children_request_handler_test.py

@@ -71,6 +71,12 @@ def enqueue_task(self, handler_selector, params):
     @patch('service_messages.send_message_notification')
     @patch('util.login_service.verify_token', return_value={'email': '[email protected]'})
     def test_put(self, verify_token, mock_method):
+
+        # Adding parent before the request to ensure overwrite the parent
+        other_inst = mocks.create_institution()
+        institution = self.inst_requested.key.get()
+        institution.set_parent(other_inst.key)
+
         """Test method post of InstitutionChildrenRequestHandler."""
         request = self.testapp.put_json(
             "/api/requests/%s/institution_children" % self.request.key.urlsafe(),
@@ -119,6 +125,11 @@ def test_put_user_not_admin(self, verify_token):
     @patch('util.login_service.verify_token', return_value={'email': '[email protected]'})
     def test_delete(self, verify_token, mock_method):
         """Test method post of InstitutionChildrenRequestHandler."""
+
+        # Adding parent before request to ensure that parent link with the institution that which invited is removed.
+        institution = self.inst_requested.key.get()
+        institution.set_parent(self.inst_test.key)
+
         self.testapp.delete(
             "/api/requests/%s/institution_children" % self.request.key.urlsafe(),
             headers={"institution-authorization": self.inst_requested.key.urlsafe()}

backend/test/institution_parent_request_handler_test.py

@@ -71,21 +71,25 @@ def enqueue_task(self, handler_selector, params):
     @patch('util.login_service.verify_token', return_value={'email': '[email protected]'})
     def test_put(self, verify_token, send_notification):
         """Test method put of InstitutionParentRequestHandler."""
+
+        # Adding child before the request to ensure that not add repeated child
+        institution = self.inst_requested.key.get()
+        institution.add_child(self.inst_test.key)
+
         request = self.testapp.put_json(
             "/api/requests/%s/institution_parent" % self.request.key.urlsafe(),
             headers={'institution-authorization': self.inst_requested.key.urlsafe()}
         )
 
         request = json.loads(request._app_iter[0])
 
-        institution = self.inst_requested.key.get()
 
         self.assertEqual(
             request['status'],
             'accepted',
             'Expected status from request must be accepted')
         self.assertEqual(
-            institution.children_institutions[0], self.inst_test.key,
+            institution.children_institutions, [self.inst_test.key],
             "The children institution of inst test must be update to inst_requested")
 
         message = {
@@ -129,6 +133,11 @@ def test_put_user_not_admin(self, verify_token):
     @patch('util.login_service.verify_token', return_value={'email': '[email protected]'})
     def test_delete(self, verify_token, send_notification):
         """Test method delete of InstitutionParentRequestHandler."""
+
+        # Adding child before the request to ensure that the existing link with the institution that which invited is removed.
+        institution = self.inst_requested.key.get()
+        institution.add_child(self.inst_test.key)
+
         self.testapp.delete(
             "/api/requests/%s/institution_parent" % self.request.key.urlsafe(),
             headers={'institution-authorization': self.inst_requested.key.urlsafe()}

backend/test/model_test/institution_test.py

@@ -42,7 +42,7 @@ def test_add_child(self):
             "Instituion should not have children"
         )
 
-        new_inst = mocks.create_institution();
+        new_inst = mocks.create_institution()
         self.institution.add_child(new_inst.key)
         self.institution = self.institution.key.get()
 
@@ -55,10 +55,27 @@ def test_add_child(self):
 
         self.assertEquals(
             self.institution.children_institutions, [new_inst.key],
-            "Instituion should have repeated children"
+            "Institution should not have repeated children"
         )    
 
-
+    def test_remove_child(self):
+
+        new_inst = mocks.create_institution()
+        self.institution.add_child(new_inst.key)
+        self.institution = self.institution.key.get()
+
+        self.assertEquals(
+            self.institution.children_institutions, [new_inst.key],
+            "Instituion should have new_inst as child"
+        )
+
+        self.institution.remove_child(new_inst.key)
+
+        self.assertEquals(
+            self.institution.children_institutions, [],
+            "Instituion should not have children"
+        )
+
     def test_follow(self):
         # case in which the user is not a follower
         self.assertEquals(self.institution.followers, [],