Skip to content

Commit

Permalink
Fix relation handling bug in generic delete action (#1275)
Browse files Browse the repository at this point in the history
  • Loading branch information
jsangmeister committed Mar 22, 2022
1 parent 07a6c62 commit af4bdfc
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 7 deletions.
1 change: 0 additions & 1 deletion openslides_backend/action/actions/motion/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ class MotionCreate(MotionCreateBase):

schema = DefaultSchema(Motion()).get_create_schema(
optional_properties=[
"title",
"number",
"state_extension",
"sort_parent_id",
Expand Down
9 changes: 3 additions & 6 deletions openslides_backend/action/generics/delete.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,13 @@ def base_update_instance(self, instance: Dict[str, Any]) -> Dict[str, Any]:
"""
Takes care of on_delete handling.
"""
# TODO: Check if instance exists in DB and is not deleted. Ensure that meta_deleted field is added to locked_fields.

# Update instance (by default this does nothing)
instance = self.update_instance(instance)

# Fetch db instance with all relevant fields
this_fqid = FullQualifiedId(self.model.collection, instance["id"])
relevant_fields = [
field.get_own_field_name()
for field in self.model.get_relation_fields()
if field.on_delete != OnDelete.SET_NULL
field.get_own_field_name() for field in self.model.get_relation_fields()
] + ["meta_deleted"]
db_instance = self.datastore.get(
fqid=this_fqid,
Expand All @@ -43,7 +39,8 @@ def base_update_instance(self, instance: Dict[str, Any]) -> Dict[str, Any]:
structured_fields += list(
self.get_all_structured_fields(field, db_instance)
)
db_instance.update(self.datastore.get(this_fqid, structured_fields))
if structured_fields:
db_instance.update(self.datastore.get(this_fqid, structured_fields))

# Update instance and set relation fields to None.
# Gather all delete actions with action data and also all models to be deleted
Expand Down
13 changes: 13 additions & 0 deletions tests/system/action/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ def create_meeting(self, base: int = 1) -> None:
"group_ids": [base, base + 1, base + 2],
"default_group_id": base,
"admin_group_id": base + 1,
"motions_default_workflow_id": base,
"committee_id": committee_id,
"is_active_in_organization_id": 1,
},
Expand All @@ -107,6 +108,17 @@ def create_meeting(self, base: int = 1) -> None:
f"group/{base+2}": {
"meeting_id": base,
},
f"motion_workflow/{base}": {
"meeting_id": base,
"default_workflow_meeting_id": base,
"state_ids": [base],
"first_state_id": base,
},
f"motion_state/{base}": {
"meeting_id": base,
"workflow_id": base,
"first_state_of_workflow_id": base,
},
f"committee/{committee_id}": {
"organization_id": 1,
"name": f"Commitee{committee_id}",
Expand All @@ -115,6 +127,7 @@ def create_meeting(self, base: int = 1) -> None:
"organization/1": {
"limit_of_meetings": 0,
"active_meeting_ids": [base],
"enable_electronic_voting": True,
},
}
)
Expand Down
85 changes: 85 additions & 0 deletions tests/system/action/user/test_delete.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,91 @@ def test_delete_with_submitter(self) -> None:
)
self.assert_model_exists("motion/50", {"submitter_ids": []})

def test_delete_with_template_field_set_null(self) -> None:
self.set_models(
{
"organization/1": {
"active_meeting_ids": [1],
"enable_electronic_voting": True,
},
"meeting/1": {
"group_ids": [1],
"default_group_id": 1,
"is_active_in_organization_id": 1,
},
"group/1": {
"meeting_id": 1,
"default_group_for_meeting_id": 1,
"user_ids": [2],
},
"user/2": {
"group_$_ids": ["1"],
"group_$1_ids": [1],
"poll_voted_$_ids": ["1"],
"poll_voted_$1_ids": [1],
},
"poll/1": {
"meeting_id": 1,
"voted_ids": [2],
},
}
)
response = self.request("user.delete", {"id": 2})
self.assert_status_code(response, 200)

self.assert_model_deleted("user/2")
self.assert_model_exists("poll/1", {"voted_ids": []})
self.assert_model_exists("group/1", {"user_ids": []})

def test_delete_with_multiple_template_fields(self) -> None:
self.set_models(
{
"organization/1": {
"active_meeting_ids": [1],
"enable_electronic_voting": True,
},
"meeting/1": {
"group_ids": [1],
"default_group_id": 1,
"is_active_in_organization_id": 1,
},
"group/1": {
"meeting_id": 1,
"default_group_for_meeting_id": 1,
"user_ids": [2],
},
"user/2": {
"group_$_ids": ["1"],
"group_$1_ids": [1],
"poll_voted_$_ids": ["1"],
"poll_voted_$1_ids": [1],
"submitted_motion_$_ids": ["1"],
"submitted_motion_$1_ids": [1],
},
"poll/1": {
"meeting_id": 1,
"voted_ids": [2],
},
"motion_submitter/1": {
"user_id": 2,
"motion_id": 1,
"meeting_id": 1,
},
"motion/1": {
"meeting_id": 1,
"submitter_ids": [1],
},
}
)
response = self.request("user.delete", {"id": 2})
self.assert_status_code(response, 200)

self.assert_model_deleted("user/2")
self.assert_model_exists("poll/1", {"voted_ids": []})
self.assert_model_exists("group/1", {"user_ids": []})
self.assert_model_deleted("motion_submitter/1")
self.assert_model_exists("motion/1", {"submitter_ids": []})

def test_delete_scope_meeting_no_permission(self) -> None:
self.setup_admin_scope_permissions(None)
self.setup_scoped_user(UserScope.Meeting)
Expand Down

0 comments on commit af4bdfc

Please sign in to comment.