Merge pull request #378 from akvo/develop

Develop

backend/db/crud_question.py

@@ -29,23 +29,27 @@ def get_question_by_commodity(
     res = []
     for param in params:
         commodity = (
-            session.query(Commodity).filter(Commodity.id == param["commodity"]).first()
+            session.query(Commodity)
+            .filter(Commodity.id == param["commodity"])
+            .first()
         )
         commodity = commodity.to_question_list
         commodity_category_id = commodity["commodity_category_id"]
         questions = (
             session.query(Question)
             .join(CommodityCategoryQuestion)
             .filter(
-                CommodityCategoryQuestion.commodity_category == commodity_category_id,
+                CommodityCategoryQuestion.commodity_category
+                == commodity_category_id,
             )
             .all()
         )
         questions = [question.serialize for question in questions]
         if not param["breakdown"]:
             questions = list(
                 filter(
-                    lambda question: question["id"] == 1 or question["parent"] == 1,
+                    lambda question: question["id"] == 1
+                    or question["parent"] == 1,
                     questions,
                 )
             )
@@ -54,9 +58,13 @@ def get_question_by_commodity(
     return res
 
 
-def get_question_by_case(session: Session, case_id: int) -> List[QuestionGroupListDict]:
+def get_question_by_case(
+    session: Session, case_id: int
+) -> List[QuestionGroupListDict]:
     case_commodity = (
-        session.query(CaseCommodity).filter(CaseCommodity.case == case_id).all()
+        session.query(CaseCommodity)
+        .filter(CaseCommodity.case == case_id)
+        .all()
     )
     commodities = (
         session.query(Commodity)
@@ -71,7 +79,8 @@ def get_question_by_case(session: Session, case_id: int) -> List[QuestionGroupLi
             session.query(Question)
             .join(CommodityCategoryQuestion)
             .filter(
-                CommodityCategoryQuestion.commodity_category == commodity_category_id,
+                CommodityCategoryQuestion.commodity_category
+                == commodity_category_id,
             )
             .all()
         )
@@ -93,3 +102,9 @@ def get_question_by_case(session: Session, case_id: int) -> List[QuestionGroupLi
         }
     )
     return res
+
+
+def get_question_by_created_by(session: Session, created_by=int):
+    return (
+        session.query(Question).filter(Question.created_by == created_by).all()
+    )

backend/db/crud_reference_data.py

@@ -171,3 +171,11 @@ def delete_reference(session: Session, id: int):
     session.delete(data)
     session.commit()
     session.flush()
+
+
+def get_reference_by_created_by(session: Session, created_by: int):
+    return (
+        session.query(ReferenceData)
+        .filter(ReferenceData.created_by == created_by)
+        .all()
+    )

backend/models/question.py

@@ -53,10 +53,14 @@ class Question(Base):
     text = Column(String, nullable=False)
     description = Column(String, nullable=True)
     default_value = Column(String, nullable=True)
-    created_by = Column(Integer, ForeignKey("user.id"), nullable=True, default=None)
+    created_by = Column(
+        Integer, ForeignKey("user.id"), nullable=True, default=None
+    )
 
     children = relationship("Question")
-    parent_detail = relationship("Question", remote_side=[id], overlaps="children")
+    parent_detail = relationship(
+        "Question", remote_side=[id], overlaps="children"
+    )
     created_by_user = relationship(
         User, cascade="all, delete", passive_deletes=True, backref="questions"
     )

backend/models/reference_data.py

@@ -137,25 +137,25 @@ def __init__(
         year: str,
         source: str,
         link: str,
-        notes: Optional[str],
-        confidence_level: Optional[str],
-        range: Optional[str],
-        area: Optional[str],
-        volume: Optional[str],
-        price: Optional[str],
-        cost_of_production: Optional[str],
-        diversified_income: Optional[str],
-        area_size_unit: Optional[str],
-        volume_measurement_unit: Optional[str],
-        cost_of_production_unit: Optional[str],
-        diversified_income_unit: Optional[str],
-        price_unit: Optional[str],
-        type_area: Optional[str],
-        type_volume: Optional[str],
-        type_price: Optional[str],
-        type_cost_of_production: Optional[str],
-        type_diversified_income: Optional[str],
-        created_by: Optional[str],
+        notes: Optional[str] = None,
+        confidence_level: Optional[str] = None,
+        range: Optional[str] = None,
+        area: Optional[str] = None,
+        volume: Optional[str] = None,
+        price: Optional[str] = None,
+        cost_of_production: Optional[str] = None,
+        diversified_income: Optional[str] = None,
+        area_size_unit: Optional[str] = None,
+        volume_measurement_unit: Optional[str] = None,
+        cost_of_production_unit: Optional[str] = None,
+        diversified_income_unit: Optional[str] = None,
+        price_unit: Optional[str] = None,
+        type_area: Optional[str] = None,
+        type_volume: Optional[str] = None,
+        type_price: Optional[str] = None,
+        type_cost_of_production: Optional[str] = None,
+        type_diversified_income: Optional[str] = None,
+        created_by: Optional[int] = None,
         id: Optional[int] = None,
     ):
         self.id = id

backend/routes/user.py

@@ -5,6 +5,8 @@
 import db.crud_case as crud_case
 import db.crud_user_case_access as crud_uca
 import db.crud_tag as crud_tag
+import db.crud_question as crud_question
+import db.crud_reference_data as crud_ref_data
 
 from math import ceil
 from middleware import (
@@ -444,19 +446,22 @@ def delete(
     if cases_owned_by_user:
         raise HTTPException(
             status_code=status.HTTP_409_CONFLICT,
-            detail=f"user {id} owned a case",
+            detail={
+                "id": user.id,
+                "email": user.email,
+                "cases": [case.to_dropdown for case in cases_owned_by_user],
+            },
         )
 
-    # Ensure that the 'case updated_by' field is moved into the case owner.
+    # Ensure that the case updated by deleted user also clean
+    # by move created_by value into updated_by column
     cases_updated_by_user = crud_case.get_case_by_updated_by(
         session=session, updated_by=user.id
     )
     if cases_updated_by_user:
         for case in cases_updated_by_user:
             case.updated_by = case.created_by
-            session.commit()
-            session.flush()
-            session.refresh(case)
+        session.commit()
 
     # Confirm that the user's case access is removed.
     crud_uca.delete_case_access_by_user_id(session=session, user_id=user.id)
@@ -471,14 +476,29 @@ def delete(
     if tags:
         for tag in tags:
             tag.created_by = None
-            session.commit()
-            session.flush()
-            session.refresh(tag)
+        session.commit()
 
-    # question created by user?
-    # reference data created by user?
+    # Verify user's ID removed from questions created_by the user
+    questions = crud_question.get_question_by_created_by(
+        session=session, created_by=user.id
+    )
+    if questions:
+        for question in questions:
+            question.created_by = None
+        session.commit()
+
+    # Verify user's ID removed from reference created_by the user
+    references = crud_ref_data.get_reference_by_created_by(
+        session=session, created_by=user.id
+    )
+    if references:
+        for reference in references:
+            reference.created_by = None
+            session.flush()
+            session.commit()
+            session.refresh(reference)
 
-    # delete user
+    # finally delete user
     crud_user.delete_user(session=session, id=user.id)
     return Response(status_code=HTTPStatus.NO_CONTENT.value)
 

backend/tests/test_1010_user_deletion.py

@@ -10,6 +10,9 @@
 from models.user_case_access import UserCaseAccess
 from models.user_business_unit import UserBusinessUnit
 from models.tag import Tag
+from models.question import Question
+from models.reference_data import ReferenceData
+from models.user import User
 
 
 pytestmark = pytest.mark.asyncio
@@ -41,6 +44,16 @@ async def test_deleting_a_user_who_has_cases(
             headers={"Authorization": f"Bearer {account.token}"},
         )
         assert res.status_code == 409
+        res = res.json()
+        assert res == {
+            "detail": {
+                "id": 1,
+                "email": "[email protected]",
+                "cases": [
+                    {"label": "Bali Coffee Production (Private)", "value": 2}
+                ],
+            }
+        }
 
     @pytest.mark.asyncio
     async def test_deleting_a_user_who_doesnt_have_cases(
@@ -100,6 +113,38 @@ async def test_deleting_a_user_who_doesnt_have_cases(
         assert tag is not None
         assert tag.created_by == deleted_user
 
+        # add question
+        question_id = 1000
+        question = Question(
+            id=question_id, text="Test Question 1000", created_by=deleted_user
+        )
+        session.add(question)
+        session.commit()
+        session.flush()
+        session.refresh(question)
+        assert question is not None
+        assert question.created_by == deleted_user
+
+        # add reference_data
+        reference_id = 10000
+        reference = ReferenceData(
+            id=reference_id,
+            country=1,
+            commodity=1,
+            region="asia",
+            currency="usd",
+            year="2020",
+            source="source",
+            link="link",
+            created_by=deleted_user,
+        )
+        session.add(reference)
+        session.commit()
+        session.flush()
+        session.refresh(reference)
+        assert reference is not None
+        assert reference.created_by == deleted_user
+
         # delete user
         res = await client.delete(
             app.url_path_for("user:delete", user_id=deleted_user),
@@ -132,3 +177,22 @@ async def test_deleting_a_user_who_doesnt_have_cases(
         # make sure tag created_by removed
         tag = session.query(Tag).filter(Tag.created_by == deleted_user).first()
         assert tag is None
+
+        # make sure question created_by is None
+        question = (
+            session.query(Question).filter(Question.id == question_id).first()
+        )
+        assert question is not None
+        assert question.id == question_id
+        assert question.created_by is None
+
+        # make sure reference data created by is None
+        reference_data = (
+            session.query(ReferenceData)
+            .filter(ReferenceData.created_by == deleted_user)
+            .all()
+        )
+        assert reference_data == []
+
+        user = session.query(User).filter(User.id == deleted_user).first()
+        assert user is None