[SYNPY-1861] remove submissionInstructionsMessage and submissionReceiptMessage from required attributes

synapseclient/models/evaluation.py

@@ -6,7 +6,7 @@
 
 from synapseclient import Synapse
 from synapseclient.core.async_utils import async_to_sync
-from synapseclient.core.utils import merge_dataclass_entities
+from synapseclient.core.utils import delete_none_keys, merge_dataclass_entities
 from synapseclient.models.protocols.evaluation_protocol import (
     EvaluationSynchronousProtocol,
 )
@@ -33,11 +33,11 @@ class Evaluation(EvaluationSynchronousProtocol):
         etag: Synapse employs an Optimistic Concurrency Control (OCC) scheme to handle concurrent updates.
               The eTag changes every time an Evaluation is updated; it is used to detect when a client's copy
               of an Evaluation is out-of-date.
-        name: The name of this Evaluation.
-        description: A text description of this Evaluation.
+        name: **Required to store.** The name of this Evaluation.
+        description: **Required to store.** A text description of this Evaluation.
         owner_id: The ID of the Synapse user who created this Evaluation.
         created_on: The date on which Evaluation was created.
-        content_source: The Synapse ID of the Entity to which this Evaluation belongs,
+        content_source: **Required to store.** The Synapse ID of the Entity to which this Evaluation belongs,
                         e.g. a reference to a Synapse project.
         submission_instructions_message: Message to display to users detailing acceptable formatting for Submissions to this Evaluation.
         submission_receipt_message: Message to display to users upon successful submission to this Evaluation.
@@ -56,8 +56,6 @@ class Evaluation(EvaluationSynchronousProtocol):
             name="My Challenge Evaluation",
             description="Evaluation for my data challenge",
             content_source="syn123456",
-            submission_instructions_message="Submit CSV files only",
-            submission_receipt_message="Thank you for your submission!",
         )
         created = evaluation.store()
         ```
@@ -123,10 +121,10 @@ class Evaluation(EvaluationSynchronousProtocol):
     of an Evaluation is out-of-date."""
 
     name: Optional[str] = None
-    """The name of this Evaluation."""
+    """**Required to store.** The name of this Evaluation."""
 
     description: Optional[str] = None
-    """A text description of this Evaluation."""
+    """**Required to store.** A text description of this Evaluation."""
 
     owner_id: Optional[str] = None
     """The ID of the Synapse user who created this Evaluation."""
@@ -135,7 +133,7 @@ class Evaluation(EvaluationSynchronousProtocol):
     """The date on which Evaluation was created."""
 
     content_source: Optional[str] = None
-    """The Synapse ID of the Entity to which this Evaluation belongs,
+    """**Required to store.** The Synapse ID of the Entity to which this Evaluation belongs,
     e.g. a reference to a Synapse project."""
 
     submission_instructions_message: Optional[str] = None
@@ -250,23 +248,26 @@ def to_synapse_request(self, request_type: RequestType):
             ValueError: If any required attributes are missing.
         """
 
-        # These attributes are required in our PUT requests for creating or updating an evaluation
-        required_attributes = [
-            "name",
-            "description",
-            "content_source",
-            "submission_instructions_message",
-            "submission_receipt_message",
-        ]
-
-        # For "update" requests, add id and etag
+        if not self.name:
+            raise ValueError(
+                f"Your evaluation object is missing the 'name' attribute. This attribute is required to {request_type.value} an evaluation"
+            )
+        if not self.description:
+            raise ValueError(
+                f"Your evaluation object is missing the 'description' attribute. This attribute is required to {request_type.value} an evaluation"
+            )
+        if not self.content_source:
+            raise ValueError(
+                f"Your evaluation object is missing the 'content_source' attribute. This attribute is required to {request_type.value} an evaluation"
+            )
         if request_type == RequestType.UPDATE:
-            required_attributes.extend(["id", "etag"])
-
-        for attribute in required_attributes:
-            if not getattr(self, attribute):
+            if not self.id:
+                raise ValueError(
+                    f"Your evaluation object is missing the 'id' attribute. This attribute is required to {request_type.value} an evaluation"
+                )
+            if not self.etag:
                 raise ValueError(
-                    f"Your evaluation object is missing the '{attribute}' attribute. This attribute is required to {request_type.value} an evaluation"
+                    f"Your evaluation object is missing the 'etag' attribute. This attribute is required to {request_type.value} an evaluation"
                 )
 
         # Build a request body for storing a brand new evaluation
@@ -277,6 +278,7 @@ def to_synapse_request(self, request_type: RequestType):
             "submissionInstructionsMessage": self.submission_instructions_message,
             "submissionReceiptMessage": self.submission_receipt_message,
         }
+        delete_none_keys(request_body)
 
         # For UPDATE request types, add id and etag
         if request_type == RequestType.UPDATE:
@@ -307,7 +309,7 @@ async def store_async(
 
         Example: Creating a new evaluation
              
-            Create a new evaluation on Synapse by storing an evaluation object with the required fields. If there are any fields missing, an error will be raised.
+            Create a new evaluation on Synapse by storing an evaluation object with the required fields.
             ```python
             from synapseclient.models import Evaluation
             from synapseclient import Synapse
@@ -321,8 +323,6 @@ async def create_evaluation():
                     name="My Challenge Evaluation",
                     description="Evaluation for my data challenge",
                     content_source="syn123456",
-                    submission_instructions_message="Submit CSV files only",
-                    submission_receipt_message="Thank you for your submission!"
                 ).store_async()
 
                 return evaluation

tests/integration/synapseclient/models/async/test_evaluation_async.py

@@ -17,18 +17,12 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None:
         self.syn = syn
         self.schedule_for_cleanup = schedule_for_cleanup
 
-    async def test_create_evaluation(self):
-        # GIVEN a project to work with
-        project = await Project(name=f"test_project_{uuid.uuid4()}").store_async(
-            synapse_client=self.syn
-        )
-        self.schedule_for_cleanup(project.id)
-
+    async def test_create_evaluation(self, project_model: Project):
         # WHEN I create an evaluation using the dataclass method
         evaluation = Evaluation(
             name=f"test_evaluation_{uuid.uuid4()}",
             description="A test evaluation for testing purposes",
-            content_source=project.id,
+            content_source=project_model.id,
             submission_instructions_message="Please submit your results in CSV format",
             submission_receipt_message="Thank you for your submission!",
         )
@@ -42,10 +36,45 @@ async def test_create_evaluation(self):
         assert (
             created_evaluation.description == "A test evaluation for testing purposes"
         )
-        assert created_evaluation.content_source == project.id
+        assert created_evaluation.content_source == project_model.id
         assert created_evaluation.owner_id is not None  # Check that owner_id is set
         assert created_evaluation.created_on is not None  # Check that created_on is set
 
+    async def test_create_evaluation_without_optional_message_fields(
+        self, project_model: Project
+    ):
+        # WHEN I create an evaluation without submission_instructions_message or submission_receipt_message
+        evaluation = Evaluation(
+            name=f"test_evaluation_{uuid.uuid4()}",
+            description="A test evaluation without optional message fields",
+            content_source=project_model.id,
+        )
+        created_evaluation = await evaluation.store_async(synapse_client=self.syn)
+        self.schedule_for_cleanup(created_evaluation.id)
+
+        # THEN the evaluation should be created successfully
+        assert created_evaluation.id is not None
+        assert created_evaluation.etag is not None
+        assert created_evaluation.name == evaluation.name
+        assert created_evaluation.content_source == project_model.id
+        assert created_evaluation.submission_instructions_message is None
+        assert created_evaluation.submission_receipt_message is None
+
+        # WHEN I update a field without setting message fields
+        new_description = f"Updated description {uuid.uuid4()}"
+        old_etag = created_evaluation.etag
+        created_evaluation.description = new_description
+        updated_evaluation = await created_evaluation.store_async(
+            synapse_client=self.syn
+        )
+
+        # THEN the update should also succeed without message fields
+        assert updated_evaluation.description == new_description
+        assert updated_evaluation.id == created_evaluation.id
+        assert updated_evaluation.etag != old_etag
+        assert updated_evaluation.submission_instructions_message is None
+        assert updated_evaluation.submission_receipt_message is None
+
 
 class TestGetEvaluation:
     @pytest.fixture(autouse=True, scope="function")
@@ -519,54 +548,3 @@ async def test_update_acl_async_with_full_dictionary(
             user_access is not None
         ), f"User {current_user_id} not found in updated ACL"
         assert set(user_access["accessType"]) == set(["READ", "DELETE", "SUBMIT"])
-
-
-class TestEvaluationValidation:
-    @pytest.fixture(autouse=True, scope="function")
-    def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None:
-        self.syn = syn
-        self.schedule_for_cleanup = schedule_for_cleanup
-
-    async def test_create_evaluation_missing_required_fields(self):
-        # WHEN I try to create an evaluation with missing required fields
-        evaluation = Evaluation(name="test_evaluation")
-
-        # THEN it should raise a ValueError
-        with pytest.raises(ValueError, match="missing the 'description' attribute"):
-            await evaluation.store_async(synapse_client=self.syn)
-
-    async def test_get_evaluation_missing_id_and_name(self):
-        # WHEN I try to get an evaluation without id or name
-        evaluation = Evaluation()
-
-        # THEN it should raise a ValueError
-        with pytest.raises(
-            ValueError, match="Either id or name must be set to get an evaluation"
-        ):
-            await evaluation.get_async(synapse_client=self.syn)
-
-    async def test_delete_evaluation_missing_id(self):
-        # WHEN I try to delete an evaluation without an id
-        evaluation = Evaluation(name="test_evaluation")
-
-        # THEN it should raise a ValueError
-        with pytest.raises(ValueError, match="id must be set to delete an evaluation"):
-            await evaluation.delete_async(synapse_client=self.syn)
-
-    async def test_get_acl_missing_id(self):
-        # WHEN I try to get ACL for an evaluation without an id
-        evaluation = Evaluation(name="test_evaluation")
-
-        # THEN it should raise a ValueError
-        with pytest.raises(ValueError, match="id must be set to get evaluation ACL"):
-            await evaluation.get_acl_async(synapse_client=self.syn)
-
-    async def test_get_permissions_missing_id(self):
-        # WHEN I try to get permissions for an evaluation without an id
-        evaluation = Evaluation(name="test_evaluation")
-
-        # THEN it should raise a ValueError
-        with pytest.raises(
-            ValueError, match="id must be set to get evaluation permissions"
-        ):
-            await evaluation.get_permissions_async(synapse_client=self.syn)

tests/integration/synapseclient/models/async/test_submission_async.py

@@ -98,6 +98,40 @@ async def test_store_submission_successfully_async(
         assert created_submission.created_on is not None
         assert created_submission.version_number is not None
 
+    async def test_store_submission_to_evaluation_without_message_fields_async(
+        self,
+        test_project: Project,
+        test_file: File,
+    ):
+        # GIVEN an evaluation created without submission_instructions_message or submission_receipt_message
+        evaluation = Evaluation(
+            name=f"test_evaluation_{uuid.uuid4()}",
+            description="Evaluation without optional message fields",
+            content_source=test_project.id,
+        )
+        created_evaluation = await evaluation.store_async(synapse_client=self.syn)
+        self.schedule_for_cleanup(created_evaluation.id)
+
+        # WHEN I submit to that evaluation
+        submission = Submission(
+            entity_id=test_file.id,
+            evaluation_id=created_evaluation.id,
+            name=f"Test Submission {uuid.uuid4()}",
+        )
+        created_submission = await submission.store_async(synapse_client=self.syn)
+        self.schedule_for_cleanup(created_submission.id)
+
+        # THEN the submission should be created successfully
+        assert created_submission.id is not None
+        assert created_submission.entity_id == test_file.id
+        assert created_submission.evaluation_id == created_evaluation.id
+
+        # AND retrieving the submission should also work
+        retrieved = await Submission(id=created_submission.id).get_async(
+            synapse_client=self.syn
+        )
+        assert retrieved.id == created_submission.id
+
     async def test_store_submission_without_entity_id_async(
         self, test_evaluation: Evaluation
     ):

tests/unit/synapseclient/models/async/unit_test_evaluation_async.py

@@ -92,7 +92,7 @@ def get_example_permissions_response(self) -> Dict[str, Union[str, List, bool]]:
         }
 
     def _build_evaluation_with_all_fields(self) -> Evaluation:
-        """Build an Evaluation instance with all required fields for store."""
+        """Build an Evaluation instance with all fields set."""
         return Evaluation(
             name=EVALUATION_NAME,
             description=EVALUATION_DESCRIPTION,