From 1e1af03d427d4b7778cfadf7413c7e32abed816a Mon Sep 17 00:00:00 2001 From: Hui Song Date: Mon, 6 Jan 2025 17:02:02 -0500 Subject: [PATCH] customize validation error messages --- src/aap_eda/api/serializers/activation.py | 23 +++++++++-- .../api/serializers/decision_environment.py | 1 + src/aap_eda/api/serializers/eda_credential.py | 9 +++++ src/aap_eda/api/serializers/event_stream.py | 7 +++- src/aap_eda/api/serializers/project.py | 8 ++++ src/aap_eda/api/serializers/team.py | 1 + tests/integration/api/test_activation.py | 38 ++++++++++++++++++- .../api/test_decision_environment.py | 19 ++++++++++ tests/integration/api/test_eda_credential.py | 20 +++++++++- tests/integration/api/test_project.py | 15 +++++++- .../services/activation/test_manager.py | 2 +- 11 files changed, 133 insertions(+), 10 deletions(-) diff --git a/src/aap_eda/api/serializers/activation.py b/src/aap_eda/api/serializers/activation.py index f2aa718a6..00262cf1b 100644 --- a/src/aap_eda/api/serializers/activation.py +++ b/src/aap_eda/api/serializers/activation.py @@ -397,9 +397,17 @@ class Meta: required=True, allow_null=False, validators=[validators.check_if_organization_exists], + error_messages={ + "null": "Organization is needed", + "required": "Organization is required", + }, ) rulebook_id = serializers.IntegerField( - validators=[validators.check_if_rulebook_exists] + validators=[validators.check_if_rulebook_exists], + error_messages={ + "null": "Rulebook is needed", + "required": "Rulebook is required", + }, ) extra_var = serializers.CharField( required=False, @@ -408,7 +416,11 @@ class Meta: validators=[validators.is_extra_var_dict], ) decision_environment_id = serializers.IntegerField( - validators=[validators.check_if_de_exists] + validators=[validators.check_if_de_exists], + error_messages={ + "null": "Decision Environment is needed", + "required": "Decision Environment is required", + }, ) user = serializers.HiddenField(default=serializers.CurrentUserDefault()) @@ -689,9 +701,12 @@ class PostActivationSerializer(serializers.ModelSerializer): required=False, allow_null=True, ) - name = serializers.CharField(required=True) + name = serializers.CharField( + required=True, error_messages={"null": "Name is needed"} + ) decision_environment_id = serializers.IntegerField( - validators=[validators.check_if_de_exists] + validators=[validators.check_if_de_exists], + error_messages={"null": "Decision Environment is needed"}, ) # TODO: is_activation_valid needs to tell event stream/activation awx_token_id = serializers.IntegerField( diff --git a/src/aap_eda/api/serializers/decision_environment.py b/src/aap_eda/api/serializers/decision_environment.py index 665ba434a..79421467e 100644 --- a/src/aap_eda/api/serializers/decision_environment.py +++ b/src/aap_eda/api/serializers/decision_environment.py @@ -44,6 +44,7 @@ class DecisionEnvironmentCreateSerializer(serializers.ModelSerializer): required=True, allow_null=False, validators=[validators.check_if_organization_exists], + error_messages={"null": "Organization is needed"}, ) eda_credential_id = serializers.IntegerField( required=False, diff --git a/src/aap_eda/api/serializers/eda_credential.py b/src/aap_eda/api/serializers/eda_credential.py index a9f1bfb47..9476619d1 100644 --- a/src/aap_eda/api/serializers/eda_credential.py +++ b/src/aap_eda/api/serializers/eda_credential.py @@ -106,11 +106,19 @@ class EdaCredentialCreateSerializer(serializers.ModelSerializer): required=True, allow_null=False, validators=[validators.check_if_credential_type_exists], + error_messages={ + "null": "Credential Type is needed", + "required": "Credential Type is required", + }, ) organization_id = serializers.IntegerField( required=True, allow_null=False, validators=[validators.check_if_organization_exists], + error_messages={ + "null": "Organization is needed", + "required": "Organization is required", + }, ) inputs = serializers.JSONField() @@ -146,6 +154,7 @@ class EdaCredentialUpdateSerializer(serializers.ModelSerializer): required=True, allow_null=False, validators=[validators.check_if_organization_exists], + error_messages={"null": "Organization is needed"}, ) inputs = serializers.JSONField() diff --git a/src/aap_eda/api/serializers/event_stream.py b/src/aap_eda/api/serializers/event_stream.py index 910ca0798..23b088356 100644 --- a/src/aap_eda/api/serializers/event_stream.py +++ b/src/aap_eda/api/serializers/event_stream.py @@ -25,7 +25,11 @@ class EventStreamInSerializer(serializers.ModelSerializer): - organization_id = serializers.IntegerField(required=True, allow_null=False) + organization_id = serializers.IntegerField( + required=True, + allow_null=False, + error_messages={"null": "Organization is needed"}, + ) owner = serializers.HiddenField(default=serializers.CurrentUserDefault()) eda_credential_id = serializers.IntegerField( required=True, @@ -33,6 +37,7 @@ class EventStreamInSerializer(serializers.ModelSerializer): validators=[ validators.check_credential_types_for_event_stream, ], + error_messages={"null": "EdaCredential is needed"}, ) def validate(self, data): diff --git a/src/aap_eda/api/serializers/project.py b/src/aap_eda/api/serializers/project.py index 6bb76ff6a..a0f04fbf7 100644 --- a/src/aap_eda/api/serializers/project.py +++ b/src/aap_eda/api/serializers/project.py @@ -74,6 +74,10 @@ class ProjectCreateRequestSerializer(serializers.ModelSerializer): required=True, allow_null=False, validators=[validators.check_if_organization_exists], + error_messages={ + "null": "Organization is needed", + "required": "Organization is required", + }, ) eda_credential_id = serializers.IntegerField( required=False, @@ -110,6 +114,10 @@ class ProjectUpdateRequestSerializer(serializers.ModelSerializer): required=True, allow_null=False, validators=[validators.check_if_organization_exists], + error_messages={ + "null": "Organization is needed", + "required": "Organization is required", + }, ) name = serializers.CharField( required=False, diff --git a/src/aap_eda/api/serializers/team.py b/src/aap_eda/api/serializers/team.py index 14aee9005..8f86eb34d 100644 --- a/src/aap_eda/api/serializers/team.py +++ b/src/aap_eda/api/serializers/team.py @@ -46,6 +46,7 @@ class TeamCreateSerializer( required=True, allow_null=False, validators=[validators.check_if_organization_exists], + error_messages={"null": "Organization is needed"}, ) class Meta: diff --git a/tests/integration/api/test_activation.py b/tests/integration/api/test_activation.py index 595c7246a..6a91b8765 100644 --- a/tests/integration/api/test_activation.py +++ b/tests/integration/api/test_activation.py @@ -558,6 +558,40 @@ def test_restart_activation( assert response.status_code == status.HTTP_204_NO_CONTENT +@pytest.mark.parametrize( + ("missing_field", "error_message"), + [ + ( + "decision_environment_id", + "Decision Environment is required", + ), + ( + "organization_id", + "Organization is required", + ), + ( + "rulebook_id", + "Rulebook is required", + ), + ], +) +@pytest.mark.django_db +def test_create_activation_with_missing_required_fields( + activation_payload: Dict[str, Any], + admin_client: APIClient, + missing_field, + error_message, + preseed_credential_types, +): + activation_payload.pop(missing_field) + response = admin_client.post( + f"{api_url_v1}/activations/", data=activation_payload + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert error_message in response.data[missing_field] + + @pytest.mark.parametrize( "enabled", [True, False], @@ -613,13 +647,13 @@ def test_restart_activation_without_de( assert response.status_code == status.HTTP_400_BAD_REQUEST assert ( response.data["errors"] - == "{'decision_environment_id': 'This field may not be null.'}" + == "{'decision_environment_id': 'Decision Environment is needed'}" ) default_activation.refresh_from_db() assert default_activation.status == enums.ActivationStatus.ERROR assert ( default_activation.status_message - == "{'decision_environment_id': 'This field may not be null.'}" + == "{'decision_environment_id': 'Decision Environment is needed'}" ) diff --git a/tests/integration/api/test_decision_environment.py b/tests/integration/api/test_decision_environment.py index b32cfba5c..24446fe67 100644 --- a/tests/integration/api/test_decision_environment.py +++ b/tests/integration/api/test_decision_environment.py @@ -322,6 +322,25 @@ def test_create_decision_environment_with_empty_credential( assert status_message in str(errors) +@pytest.mark.django_db +def test_create_decision_environment_with_none_organization( + admin_client: APIClient, +): + data_in = { + "name": "de1", + "description": "desc here", + "image_url": "registry.com/img1:tag1", + "organization_id": None, + } + + response = admin_client.post( + f"{api_url_v1}/decision-environments/", data=data_in + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "Organization is needed" in str(response.data) + + @pytest.mark.django_db def test_create_decision_environment_bad_ids(admin_client: APIClient): bad_ids = [ diff --git a/tests/integration/api/test_eda_credential.py b/tests/integration/api/test_eda_credential.py index 712d9f04d..dec69690d 100644 --- a/tests/integration/api/test_eda_credential.py +++ b/tests/integration/api/test_eda_credential.py @@ -161,7 +161,25 @@ def test_create_eda_credential_with_none_credential_type( f"{api_url_v1}/eda-credentials/", data=data_in ) assert response.status_code == status.HTTP_400_BAD_REQUEST - assert "This field may not be null." in response.data["credential_type_id"] + assert "Credential Type is needed" in response.data["credential_type_id"] + + +@pytest.mark.django_db +def test_create_eda_credential_with_none_organization( + admin_client: APIClient, +): + data = "secret" + data_in = { + "name": "eda-credential", + "inputs": {"username": "adam", "password": data}, + "credential_type_id": None, + "organization_id": None, + } + response = admin_client.post( + f"{api_url_v1}/eda-credentials/", data=data_in + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "Organization is needed" in response.data["organization_id"] @pytest.mark.parametrize( diff --git a/tests/integration/api/test_project.py b/tests/integration/api/test_project.py index a3acbcfd4..d107d9371 100644 --- a/tests/integration/api/test_project.py +++ b/tests/integration/api/test_project.py @@ -393,6 +393,19 @@ def test_create_or_update_project_with_right_eda_credential( ) +@pytest.mark.django_db +def test_create_project_with_none_organization(admin_client: APIClient): + body = { + "name": "none-organization", + "url": "https://git.example.com/acme/project-01", + "organization_id": None, + } + + response = admin_client.post(f"{api_url_v1}/projects/", data=body) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "Organization is needed" in str(response.data) + + @pytest.mark.django_db def test_create_project_name_conflict( default_project: models.Project, admin_client: APIClient @@ -689,7 +702,7 @@ def test_partial_update_project_null_organization_id( data, ) assert response.status_code == status.HTTP_400_BAD_REQUEST - assert "This field may not be null." in str(response.data) + assert "Organization is needed" in str(response.data) @pytest.mark.django_db diff --git a/tests/integration/services/activation/test_manager.py b/tests/integration/services/activation/test_manager.py index 3d36c790b..e1951ba00 100644 --- a/tests/integration/services/activation/test_manager.py +++ b/tests/integration/services/activation/test_manager.py @@ -293,7 +293,7 @@ def test_start_no_decision_environment( activation_manager.start() assert basic_activation.status == enums.ActivationStatus.ERROR assert "decision_environment" in str(exc.value) - assert "This field may not be null" in str(exc.value) + assert "Decision Environment is needed" in str(exc.value) assert str(exc.value) in basic_activation.status_message