Merge pull request #498 from biocore/csymons_delete_fix_new

Resolve external survey account deletion bug.

microsetta_private_api/admin/admin_impl.py

@@ -808,10 +808,17 @@ def delete_account(account_id, token_info):
 
         sample_count = 0
         account_has_external = False
-        sources = src_repo.get_sources_in_account(account_id)
+        sources = src_repo.get_sources_in_account(
+            account_id,
+            allow_revoked=True
+        )
 
         for source in sources:
-            samples = samp_repo.get_samples_by_source(account_id, source.id)
+            samples = samp_repo.get_samples_by_source(
+                account_id,
+                source.id,
+                allow_revoked=True
+            )
 
             has_samples = len(samples) > 0
             sample_count += len(samples)
@@ -832,7 +839,11 @@ def delete_account(account_id, token_info):
                 # survey / source free text
                 for survey_id in surveys:
                     sar_repo.scrub(account_id, source.id, survey_id)
-                src_repo.scrub(account_id, source.id)
+
+                # We're including scrubbed sources to detect external surveys
+                # so we need to make sure the source isn't already scrubbed
+                if source.source_data.date_revoked is None:
+                    src_repo.scrub(account_id, source.id)
 
             if not has_samples and not has_external:
                 # if we do not have associated samples, or external surveys,

microsetta_private_api/api/_source.py

@@ -4,11 +4,14 @@
 from flask import jsonify
 
 from microsetta_private_api.api._account import _validate_account_access
-from microsetta_private_api.api.literals import SRC_NOT_FOUND_MSG
+from microsetta_private_api.api.literals import SRC_NOT_FOUND_MSG,\
+    SRC_NO_DELETE_MSG
 from microsetta_private_api.exceptions import RepoException
 from microsetta_private_api.model.source import Source, HumanInfo, NonHumanInfo
 from microsetta_private_api.repo.source_repo import SourceRepo
 from microsetta_private_api.repo.survey_answers_repo import SurveyAnswersRepo
+from microsetta_private_api.repo.survey_template_repo import SurveyTemplateRepo
+from microsetta_private_api.repo.sample_repo import SampleRepo
 from microsetta_private_api.repo.transaction import Transaction
 
 
@@ -109,18 +112,47 @@ def delete_source(account_id, source_id, token_info):
     with Transaction() as t:
         source_repo = SourceRepo(t)
         survey_answers_repo = SurveyAnswersRepo(t)
-
-        answers = survey_answers_repo.list_answered_surveys(account_id,
-                                                            source_id)
-        for survey_id in answers:
-            survey_answers_repo.delete_answered_survey(account_id,
-                                                       survey_id)
-
-        if not source_repo.delete_source(account_id, source_id):
-            return jsonify(code=404, message=SRC_NOT_FOUND_MSG), 404
-        # TODO: 422?
-        t.commit()
-        return '', 204
+        template_repo = SurveyTemplateRepo(t)
+        sample_repo = SampleRepo(t)
+
+        # The interface has historically enforced this constraint, but it
+        # wasn't codified into the API
+        samples = sample_repo.get_samples_by_source(account_id, source_id)
+        if len(samples) > 0:
+            return jsonify(code=422, message=SRC_NO_DELETE_MSG), 422
+        else:
+            has_external = template_repo.has_external_surveys(
+                account_id,
+                source_id
+            )
+            answers = survey_answers_repo.list_answered_surveys(
+                account_id,
+                source_id
+            )
+
+            if has_external:
+                for survey_id in answers:
+                    survey_answers_repo.scrub(
+                        account_id,
+                        source_id,
+                        survey_id
+                    )
+                if not source_repo.scrub(account_id, source_id):
+                    return jsonify(code=404, message=SRC_NOT_FOUND_MSG), 404
+            else:
+                # If we reach this point, then the user does not have external
+                # surveys nor do they have samples associated. Therefore it is
+                # safe to remove their surveys and source entirely
+                for survey_id in answers:
+                    survey_answers_repo.delete_answered_survey(
+                        account_id,
+                        survey_id
+                    )
+                if not source_repo.delete_source(account_id, source_id):
+                    return jsonify(code=404, message=SRC_NOT_FOUND_MSG), 404
+
+            t.commit()
+            return '', 204
 
 
 def create_human_source_from_consent(account_id, body, token_info):

microsetta_private_api/api/literals.py

@@ -13,4 +13,6 @@
 JWT_EMAIL_CLAIM_KEY = 'email'
 ACCT_NOT_FOUND_MSG = "Account not found"
 SRC_NOT_FOUND_MSG = "Source not found"
+SRC_NO_DELETE_MSG = "A source cannot be deleted while samples are "\
+    "associated with it"
 INVALID_TOKEN_MSG = "Invalid token"

microsetta_private_api/api/tests/test_api.py

@@ -1002,6 +1002,53 @@ def test_account_scrub_success(self):
         self.assertEqual(response_obj['sample_datetime'],
                          sample_body['sample_datetime'][:-1])
 
+    # This test specifically verifies that the scenario in Private API
+    # issue #492 - where a user takes an external survey, deletes the source,
+    # then requests account deletion - is resolved
+    def test_account_scrub_success_external_surveys_deleted_source(self):
+        # setup an account with a source and sample
+        dummy_acct_id, dummy_source_id = create_dummy_source(
+            "Bo", Source.SOURCE_TYPE_HUMAN, DUMMY_HUMAN_SOURCE,
+            create_dummy_1=True)
+
+        _ = create_dummy_acct(create_dummy_1=False,
+                              iss=ACCT_MOCK_ISS_3,
+                              sub=ACCT_MOCK_SUB_3,
+                              dummy_is_admin=True)
+
+        # "take" the polyphenol FFQ
+        response = self.client.get(
+            '/api/accounts/%s/sources/%s/survey_templates/%s?language_tag=en_us' %  # noqa
+            (dummy_acct_id, dummy_source_id,
+             SurveyTemplateRepo.POLYPHENOL_FFQ_ID),
+            headers=self.dummy_auth)
+
+        # delete the source
+        response = self.client.delete(
+            '/api/accounts/%s/sources/%s?language_tag=en_us' %
+            (dummy_acct_id, dummy_source_id),
+            headers=self.dummy_auth
+        )
+
+        # now let's scrub the account
+        response = self.client.delete(
+            '/api/accounts/%s' %
+            (dummy_acct_id,),
+            headers=make_headers(FAKE_TOKEN_ADMIN))
+
+        # check response code
+        self.assertEqual(204, response.status_code)
+
+        # verify deleting is idempotent. If the account was scrubbed, then
+        # we get a 204. If the account was deleted, we get a 404
+        response = self.client.delete(
+            '/api/accounts/%s' %
+            (dummy_acct_id,),
+            headers=make_headers(FAKE_TOKEN_ADMIN))
+
+        # check response code
+        self.assertEqual(204, response.status_code)
+
     def test_account_delete_without_samples(self):
         # setup an account with a source and sample
         dummy_acct_id, dummy_source_id = create_dummy_source(

microsetta_private_api/db/patches/0112.sql

@@ -0,0 +1,4 @@
+-- We need to clean up stale state on an account that requested deletion.
+-- Using a non-sequential patch filename to ease merging master and master-overhaul soon.
+DELETE FROM ag.polyphenol_ffq_registry WHERE account_id = 'b499417a-0840-44da-b269-d243f6eb02ae';
+DELETE FROM ag.spain_ffq_registry WHERE account_id = 'b499417a-0840-44da-b269-d243f6eb02ae';