From 390c6ca088e3a564e950040c62d54604117be34f Mon Sep 17 00:00:00 2001
From: Jenny Medina <medina.stsci@gmail.com>
Date: Thu, 26 Sep 2024 14:39:59 -0400
Subject: [PATCH 01/16] New validate_submission_id for methods that retrieve
 submissions. New unit tests.

---
 synapseclient/client.py                       |  9 +++---
 synapseclient/core/utils.py                   | 26 +++++++++++++++
 .../synapseclient/core/unit_test_utils.py     |  7 ++++
 tests/unit/synapseclient/unit_test_client.py  | 32 +++++++++++++++++++
 4 files changed, 70 insertions(+), 4 deletions(-)

diff --git a/synapseclient/client.py b/synapseclient/client.py
index ce25aa5ab..af0823955 100644
--- a/synapseclient/client.py
+++ b/synapseclient/client.py
@@ -114,6 +114,7 @@
     is_integer,
     is_json,
     require_param,
+    validate_submission_id,
 )
 from synapseclient.core.version_check import version_check
 
@@ -4807,7 +4808,7 @@ def _POST_paginated(self, uri: str, body, **kwargs):
             if next_page_token is None:
                 break
 
-    def getSubmission(self, id, **kwargs):
+    def getSubmission(self, id: typing.Union[str, int], **kwargs) -> Submission:
         """
         Gets a [synapseclient.evaluation.Submission][] object by its id.
 
@@ -4823,7 +4824,7 @@ def getSubmission(self, id, **kwargs):
              on the *downloadFile*, *downloadLocation*, and *ifcollision* parameters
         """
 
-        submission_id = id_of(id)
+        submission_id = validate_submission_id(id)
         uri = Submission.getURI(submission_id)
         submission = Submission(**self.restGET(uri))
 
@@ -4852,7 +4853,7 @@ def getSubmission(self, id, **kwargs):
 
         return submission
 
-    def getSubmissionStatus(self, submission):
+    def getSubmissionStatus(self, submission: typing.Union[str, int]) -> SubmissionStatus:
         """
         Downloads the status of a Submission.
 
@@ -4863,7 +4864,7 @@ def getSubmissionStatus(self, submission):
             A [synapseclient.evaluation.SubmissionStatus][] object
         """
 
-        submission_id = id_of(submission)
+        submission_id = validate_submission_id(submission)
         uri = SubmissionStatus.getURI(submission_id)
         val = self.restGET(uri)
         return SubmissionStatus(**val)
diff --git a/synapseclient/core/utils.py b/synapseclient/core/utils.py
index 7af693729..2bc37d1e8 100644
--- a/synapseclient/core/utils.py
+++ b/synapseclient/core/utils.py
@@ -242,6 +242,32 @@ def id_of(obj: typing.Union[str, collections.abc.Mapping, numbers.Number]) -> st
     raise ValueError("Invalid parameters: couldn't find id of " + str(obj))
 
 
+def validate_submission_id(submission_id: typing.Union[str, int]) -> str:
+    """
+    Ensures that a given submission ID is either an integer or a string that
+    can be converted to an integer. Version notation is not supported for submission
+    IDs, therefore decimals are not allowed.
+
+    Arguments:
+        submission_id: The submission ID to validate
+
+    Returns:
+        The submission ID as a string
+
+    Raises:
+        ValueError: if the submission ID is invalid
+
+    """
+    if isinstance(submission_id, int):
+        return str(submission_id)
+    elif isinstance(submission_id, str) and submission_id.isdigit():
+        return submission_id
+    else:
+        raise ValueError(
+            f"Invalid submission ID: {submission_id}. ID can either be an integer or a string with no decimals."
+        )
+
+
 def concrete_type_of(obj: collections.abc.Mapping):
     """
     Return the concrete type of an object representing a Synapse entity.
diff --git a/tests/unit/synapseclient/core/unit_test_utils.py b/tests/unit/synapseclient/core/unit_test_utils.py
index 5b521bcea..a96a62df7 100644
--- a/tests/unit/synapseclient/core/unit_test_utils.py
+++ b/tests/unit/synapseclient/core/unit_test_utils.py
@@ -100,6 +100,13 @@ def __init__(self, id_attr_name: str, id: str) -> None:
         assert utils.id_of(foo) == "123"
 
 
+def test_validate_submission_id() -> None:
+    assert utils.validate_submission_id("123") == "123"
+    assert utils.validate_submission_id(123) == "123"
+    pytest.raises(ValueError, utils.validate_submission_id, "123.0")
+    pytest.raises(ValueError, utils.validate_submission_id, 123.0)
+
+
 # TODO: Add a test for is_synapse_id_str(...)
 # https://sagebionetworks.jira.com/browse/SYNPY-1425
 
diff --git a/tests/unit/synapseclient/unit_test_client.py b/tests/unit/synapseclient/unit_test_client.py
index 3489d9526..fed7974bd 100644
--- a/tests/unit/synapseclient/unit_test_client.py
+++ b/tests/unit/synapseclient/unit_test_client.py
@@ -2995,6 +2995,38 @@ def test_get_submission_with_annotations(syn: Synapse) -> None:
         assert evaluation_id == response["evaluationId"]
 
 
+@pytest.mark.parametrize("submission_id", ["123", 123])
+def test_get_submission_valid_id(syn: Synapse, submission_id) -> None:
+    evaluation_id = (98765,)
+
+    submission = {
+        "evaluationId": evaluation_id,
+        "entityId": submission_id,
+        "versionNumber": 1,
+        "entityBundleJSON": json.dumps({}),
+    }
+    with patch.object(syn, "restGET") as restGET, patch.object(
+        syn, "_getWithEntityBundle"
+    ) as get_entity:
+        restGET.return_value = submission
+        syn.getSubmission(submission_id)
+        restGET.assert_called_once_with(f"/evaluation/submission/{submission_id}")
+        get_entity.assert_called_once_with(
+            entityBundle={},
+            entity=submission_id,
+            submission=str(submission_id),
+        )
+
+
+@pytest.mark.parametrize("submission_id", ["123.0", 123.0])
+def test_get_submission_invalid_id(syn: Synapse, submission_id) -> None:
+    with pytest.raises(
+        ValueError,
+        match=f"Invalid submission ID: {submission_id}. ID can either be an integer or a string with no decimals.",
+    ):
+        syn.getSubmission(submission_id)
+
+
 class TestTableSnapshot:
     def test__create_table_snapshot(self, syn: Synapse) -> None:
         """Testing creating table snapshots"""

From 9bbaf666a8ce96b6758c258d6f014ff6f803531a Mon Sep 17 00:00:00 2001
From: Jenny Medina <medina.stsci@gmail.com>
Date: Thu, 26 Sep 2024 14:51:10 -0400
Subject: [PATCH 02/16] Style

---
 synapseclient/client.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/synapseclient/client.py b/synapseclient/client.py
index af0823955..1b1a19228 100644
--- a/synapseclient/client.py
+++ b/synapseclient/client.py
@@ -4853,7 +4853,9 @@ def getSubmission(self, id: typing.Union[str, int], **kwargs) -> Submission:
 
         return submission
 
-    def getSubmissionStatus(self, submission: typing.Union[str, int]) -> SubmissionStatus:
+    def getSubmissionStatus(
+        self, submission: typing.Union[str, int]
+    ) -> SubmissionStatus:
         """
         Downloads the status of a Submission.
 

From 1eb752b90aa804818e02f5195d3e7bd60c40e469 Mon Sep 17 00:00:00 2001
From: Jenny Medina <medina.stsci@gmail.com>
Date: Fri, 27 Sep 2024 01:07:08 -0400
Subject: [PATCH 03/16] Update tests to reflect new functionality

---
 tests/integration/synapseclient/test_evaluations.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/integration/synapseclient/test_evaluations.py b/tests/integration/synapseclient/test_evaluations.py
index 4f752c244..8e057d2f8 100644
--- a/tests/integration/synapseclient/test_evaluations.py
+++ b/tests/integration/synapseclient/test_evaluations.py
@@ -66,7 +66,7 @@ async def test_evaluations(syn: Synapse, project: Project):
             assert p in permissions
 
         # Test getSubmissions with no Submissions (SYNR-453)
-        submissions = syn.getSubmissions(ev)
+        submissions = syn.getSubmissions(ev.get("id"))
         assert len(list(submissions)) == 0
 
         # Increase this to fully test paging by getEvaluationSubmissions
@@ -107,10 +107,10 @@ async def test_evaluations(syn: Synapse, project: Project):
         ]
 
         # Score the submissions
-        submissions = syn.getSubmissions(ev, limit=num_of_submissions - 1)
+        submissions = syn.getSubmissions(ev.get("id"), limit=num_of_submissions - 1)
         for submission in submissions:
             assert re.match("Submission \\d+", submission["name"])
-            status = syn.getSubmissionStatus(submission)
+            status = syn.getSubmissionStatus(submission.get("id"))
             if submission["name"] == "Submission 01":
                 status.status = "INVALID"
             else:
@@ -119,7 +119,7 @@ async def test_evaluations(syn: Synapse, project: Project):
 
         # Annotate the submissions
         bogosity = {}
-        submissions = syn.getSubmissions(ev)
+        submissions = syn.getSubmissions(ev.get("id"))
         b = 123
         for submission, status in syn.getSubmissionBundles(ev):
             bogosity[submission.id] = b

From e16a56260eb61ff595cde7b726d6baa5217a610f Mon Sep 17 00:00:00 2001
From: Jenny Medina <medina.stsci@gmail.com>
Date: Fri, 27 Sep 2024 02:00:03 -0400
Subject: [PATCH 04/16] getSubmissions did not need to be fed the ID

---
 tests/integration/synapseclient/test_evaluations.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/integration/synapseclient/test_evaluations.py b/tests/integration/synapseclient/test_evaluations.py
index 8e057d2f8..e80c71b3e 100644
--- a/tests/integration/synapseclient/test_evaluations.py
+++ b/tests/integration/synapseclient/test_evaluations.py
@@ -66,7 +66,7 @@ async def test_evaluations(syn: Synapse, project: Project):
             assert p in permissions
 
         # Test getSubmissions with no Submissions (SYNR-453)
-        submissions = syn.getSubmissions(ev.get("id"))
+        submissions = syn.getSubmissions(ev)
         assert len(list(submissions)) == 0
 
         # Increase this to fully test paging by getEvaluationSubmissions
@@ -107,7 +107,7 @@ async def test_evaluations(syn: Synapse, project: Project):
         ]
 
         # Score the submissions
-        submissions = syn.getSubmissions(ev.get("id"), limit=num_of_submissions - 1)
+        submissions = syn.getSubmissions(ev, limit=num_of_submissions - 1)
         for submission in submissions:
             assert re.match("Submission \\d+", submission["name"])
             status = syn.getSubmissionStatus(submission.get("id"))
@@ -119,7 +119,7 @@ async def test_evaluations(syn: Synapse, project: Project):
 
         # Annotate the submissions
         bogosity = {}
-        submissions = syn.getSubmissions(ev.get("id"))
+        submissions = syn.getSubmissions(ev)
         b = 123
         for submission, status in syn.getSubmissionBundles(ev):
             bogosity[submission.id] = b

From aa632fbc3ffd8c7c9d42e64785f1134034effcd8 Mon Sep 17 00:00:00 2001
From: Jenny Medina <medina.stsci@gmail.com>
Date: Fri, 27 Sep 2024 11:10:57 -0400
Subject: [PATCH 05/16] Pass and transform invalid input, with warning. Update
 utils unit test

---
 synapseclient/core/utils.py                      | 16 +++++++++++-----
 tests/unit/synapseclient/core/unit_test_utils.py | 14 +++++++++++---
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/synapseclient/core/utils.py b/synapseclient/core/utils.py
index 2bc37d1e8..e78b0f317 100644
--- a/synapseclient/core/utils.py
+++ b/synapseclient/core/utils.py
@@ -11,6 +11,7 @@
 import hashlib
 import importlib
 import inspect
+import logging
 import numbers
 import os
 import platform
@@ -26,6 +27,7 @@
 import zipfile
 from dataclasses import asdict, is_dataclass
 from typing import TYPE_CHECKING, TypeVar
+from synapseclient.core.logging_setup import DEFAULT_LOGGER_NAME
 
 import requests
 from opentelemetry import trace
@@ -47,6 +49,10 @@
 
 SLASH_PREFIX_REGEX = re.compile(r"\/[A-Za-z]:")
 
+# Set up logging
+LOGGER_NAME = DEFAULT_LOGGER_NAME
+LOGGER = logging.getLogger(LOGGER_NAME)
+logging.getLogger("py.warnings").handlers = LOGGER.handlers
 
 def md5_for_file(
     filename: str, block_size: int = 2 * MB, callback: typing.Callable = None
@@ -254,18 +260,18 @@ def validate_submission_id(submission_id: typing.Union[str, int]) -> str:
     Returns:
         The submission ID as a string
 
-    Raises:
-        ValueError: if the submission ID is invalid
-
     """
     if isinstance(submission_id, int):
         return str(submission_id)
     elif isinstance(submission_id, str) and submission_id.isdigit():
         return submission_id
     else:
-        raise ValueError(
-            f"Invalid submission ID: {submission_id}. ID can either be an integer or a string with no decimals."
+        int_submission_id = int(float(submission_id))
+        LOGGER.warning(
+            f"Submission ID '{submission_id}' contains decimals which are not supported. "
+            f"Submission ID will be converted to '{int_submission_id}'."
         )
+        return str(int_submission_id)
 
 
 def concrete_type_of(obj: collections.abc.Mapping):
diff --git a/tests/unit/synapseclient/core/unit_test_utils.py b/tests/unit/synapseclient/core/unit_test_utils.py
index a96a62df7..07ad05ff0 100644
--- a/tests/unit/synapseclient/core/unit_test_utils.py
+++ b/tests/unit/synapseclient/core/unit_test_utils.py
@@ -1,6 +1,7 @@
 # unit tests for utils.py
 
 import base64
+import logging
 import os
 import re
 import tempfile
@@ -100,11 +101,18 @@ def __init__(self, id_attr_name: str, id: str) -> None:
         assert utils.id_of(foo) == "123"
 
 
-def test_validate_submission_id() -> None:
+def test_validate_submission_id(caplog) -> None:
+    # Test 1: Test valid inputs
     assert utils.validate_submission_id("123") == "123"
     assert utils.validate_submission_id(123) == "123"
-    pytest.raises(ValueError, utils.validate_submission_id, "123.0")
-    pytest.raises(ValueError, utils.validate_submission_id, 123.0)
+    
+    # Test 2: Test invalid inputs get corrected
+    with caplog.at_level(logging.WARNING):
+        assert utils.validate_submission_id("123.0") == "123"
+        assert "Submission ID '123.0' contains decimals which are not supported" in caplog.text
+    with caplog.at_level(logging.WARNING):
+        assert utils.validate_submission_id(123.0) == "123"
+        assert "Submission ID '123.0' contains decimals which are not supported" in caplog.text
 
 
 # TODO: Add a test for is_synapse_id_str(...)

From 2daa979771162b0de9d01b838edb1769d498c73d Mon Sep 17 00:00:00 2001
From: Jenny Medina <medina.stsci@gmail.com>
Date: Fri, 27 Sep 2024 11:31:29 -0400
Subject: [PATCH 06/16] Updated getSubmission() unit tests

---
 tests/unit/synapseclient/unit_test_client.py | 57 ++++++++++++++------
 1 file changed, 41 insertions(+), 16 deletions(-)

diff --git a/tests/unit/synapseclient/unit_test_client.py b/tests/unit/synapseclient/unit_test_client.py
index fed7974bd..b6374b729 100644
--- a/tests/unit/synapseclient/unit_test_client.py
+++ b/tests/unit/synapseclient/unit_test_client.py
@@ -9,6 +9,7 @@
 import tempfile
 import urllib.request as urllib_request
 import uuid
+import typing
 from pathlib import Path
 from unittest.mock import ANY, AsyncMock, MagicMock, Mock, call, create_autospec, patch
 
@@ -2995,36 +2996,60 @@ def test_get_submission_with_annotations(syn: Synapse) -> None:
         assert evaluation_id == response["evaluationId"]
 
 
-@pytest.mark.parametrize("submission_id", ["123", 123])
-def test_get_submission_valid_id(syn: Synapse, submission_id) -> None:
-    evaluation_id = (98765,)
+def run_submission_test(syn: Synapse, submission_id: typing.Union[str, int], expected_id: str, should_warn: bool = False, caplog=None) -> None:
+    """
+    Common code for test_get_submission_valid_id and test_get_submission_invalid_id.
+    Generates a dummy submission dictionary for regression testing, mocks the API calls,
+    and validates the expected output for getSubmission. For invalid submission IDs, this
+    will check that a warning was logged for the user before transforming their input.
+
+    Arguments:
+        syn: Synapse object
+        submission_id: Submission ID to test
+        expected_id: Submission ID that should be returned
+        should_warn: Whether or not a warning should be logged
+        caplog: pytest caplog fixture
 
+    Returns:
+        None
+
+    """
+    evaluation_id = (98765,)
     submission = {
         "evaluationId": evaluation_id,
         "entityId": submission_id,
         "versionNumber": 1,
         "entityBundleJSON": json.dumps({}),
     }
-    with patch.object(syn, "restGET") as restGET, patch.object(
-        syn, "_getWithEntityBundle"
-    ) as get_entity:
+
+    with patch.object(syn, "restGET") as restGET, patch.object(syn, "_getWithEntityBundle") as get_entity:
         restGET.return_value = submission
-        syn.getSubmission(submission_id)
-        restGET.assert_called_once_with(f"/evaluation/submission/{submission_id}")
+
+        if should_warn:
+            with caplog.at_level(logging.WARNING):
+                syn.getSubmission(submission_id)
+                assert f"Submission ID '{submission_id}' contains decimals which are not supported" in caplog.text
+        else:
+            syn.getSubmission(submission_id)
+
+        restGET.assert_called_once_with(f"/evaluation/submission/{expected_id}")
         get_entity.assert_called_once_with(
             entityBundle={},
             entity=submission_id,
-            submission=str(submission_id),
+            submission=str(expected_id),
         )
 
 
-@pytest.mark.parametrize("submission_id", ["123.0", 123.0])
-def test_get_submission_invalid_id(syn: Synapse, submission_id) -> None:
-    with pytest.raises(
-        ValueError,
-        match=f"Invalid submission ID: {submission_id}. ID can either be an integer or a string with no decimals.",
-    ):
-        syn.getSubmission(submission_id)
+@pytest.mark.parametrize("submission_id, expected_id", [("123", "123"), (123, "123")])
+def test_get_submission_valid_id(syn: Synapse, submission_id, expected_id) -> None:
+    """ Test getSubmission with valid submission ID """
+    run_submission_test(syn, submission_id, expected_id)
+
+
+@pytest.mark.parametrize("submission_id, expected_id", [("123.0", "123"), (123.0, "123")])
+def test_get_submission_invalid_id(syn: Synapse, submission_id, expected_id, caplog) -> None:
+    """ Test getSubmission with invalid submission ID """
+    run_submission_test(syn, submission_id, expected_id, should_warn=True, caplog=caplog)
 
 
 class TestTableSnapshot:

From 20993078dbcff8dfc69a429b777d32d14f9e80fa Mon Sep 17 00:00:00 2001
From: Jenny Medina <medina.stsci@gmail.com>
Date: Fri, 27 Sep 2024 11:32:57 -0400
Subject: [PATCH 07/16] Revert change made in test_evaluations.py

---
 tests/integration/synapseclient/test_evaluations.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/integration/synapseclient/test_evaluations.py b/tests/integration/synapseclient/test_evaluations.py
index e80c71b3e..4f752c244 100644
--- a/tests/integration/synapseclient/test_evaluations.py
+++ b/tests/integration/synapseclient/test_evaluations.py
@@ -110,7 +110,7 @@ async def test_evaluations(syn: Synapse, project: Project):
         submissions = syn.getSubmissions(ev, limit=num_of_submissions - 1)
         for submission in submissions:
             assert re.match("Submission \\d+", submission["name"])
-            status = syn.getSubmissionStatus(submission.get("id"))
+            status = syn.getSubmissionStatus(submission)
             if submission["name"] == "Submission 01":
                 status.status = "INVALID"
             else:

From c33f42448868ac9f88439f2745e72ca9d8be8ff0 Mon Sep 17 00:00:00 2001
From: Jenny Medina <medina.stsci@gmail.com>
Date: Fri, 27 Sep 2024 11:50:15 -0400
Subject: [PATCH 08/16] Support mapping objects

---
 synapseclient/client.py                       |  4 ++-
 synapseclient/core/utils.py                   | 12 +++++--
 .../synapseclient/core/unit_test_utils.py     | 12 +++++--
 tests/unit/synapseclient/unit_test_client.py  | 35 ++++++++++++++-----
 4 files changed, 48 insertions(+), 15 deletions(-)

diff --git a/synapseclient/client.py b/synapseclient/client.py
index 1b1a19228..c38d72dcb 100644
--- a/synapseclient/client.py
+++ b/synapseclient/client.py
@@ -4808,7 +4808,9 @@ def _POST_paginated(self, uri: str, body, **kwargs):
             if next_page_token is None:
                 break
 
-    def getSubmission(self, id: typing.Union[str, int], **kwargs) -> Submission:
+    def getSubmission(
+        self, id: typing.Union[str, int, collections.abc.Mapping], **kwargs
+    ) -> Submission:
         """
         Gets a [synapseclient.evaluation.Submission][] object by its id.
 
diff --git a/synapseclient/core/utils.py b/synapseclient/core/utils.py
index e78b0f317..8bbe4b2df 100644
--- a/synapseclient/core/utils.py
+++ b/synapseclient/core/utils.py
@@ -27,11 +27,12 @@
 import zipfile
 from dataclasses import asdict, is_dataclass
 from typing import TYPE_CHECKING, TypeVar
-from synapseclient.core.logging_setup import DEFAULT_LOGGER_NAME
 
 import requests
 from opentelemetry import trace
 
+from synapseclient.core.logging_setup import DEFAULT_LOGGER_NAME
+
 if TYPE_CHECKING:
     from synapseclient.models import File, Folder, Project
 
@@ -54,6 +55,7 @@
 LOGGER = logging.getLogger(LOGGER_NAME)
 logging.getLogger("py.warnings").handlers = LOGGER.handlers
 
+
 def md5_for_file(
     filename: str, block_size: int = 2 * MB, callback: typing.Callable = None
 ):
@@ -248,7 +250,9 @@ def id_of(obj: typing.Union[str, collections.abc.Mapping, numbers.Number]) -> st
     raise ValueError("Invalid parameters: couldn't find id of " + str(obj))
 
 
-def validate_submission_id(submission_id: typing.Union[str, int]) -> str:
+def validate_submission_id(
+    submission_id: typing.Union[str, int, collections.abc.Mapping]
+) -> str:
     """
     Ensures that a given submission ID is either an integer or a string that
     can be converted to an integer. Version notation is not supported for submission
@@ -265,6 +269,10 @@ def validate_submission_id(submission_id: typing.Union[str, int]) -> str:
         return str(submission_id)
     elif isinstance(submission_id, str) and submission_id.isdigit():
         return submission_id
+    elif isinstance(submission_id, collections.abc.Mapping):
+        syn_id = _get_from_members_items_or_properties(submission_id, "id")
+        if syn_id is not None:
+            return str((float(syn_id)))
     else:
         int_submission_id = int(float(submission_id))
         LOGGER.warning(
diff --git a/tests/unit/synapseclient/core/unit_test_utils.py b/tests/unit/synapseclient/core/unit_test_utils.py
index 07ad05ff0..320e1d131 100644
--- a/tests/unit/synapseclient/core/unit_test_utils.py
+++ b/tests/unit/synapseclient/core/unit_test_utils.py
@@ -105,14 +105,20 @@ def test_validate_submission_id(caplog) -> None:
     # Test 1: Test valid inputs
     assert utils.validate_submission_id("123") == "123"
     assert utils.validate_submission_id(123) == "123"
-    
+
     # Test 2: Test invalid inputs get corrected
     with caplog.at_level(logging.WARNING):
         assert utils.validate_submission_id("123.0") == "123"
-        assert "Submission ID '123.0' contains decimals which are not supported" in caplog.text
+        assert (
+            "Submission ID '123.0' contains decimals which are not supported"
+            in caplog.text
+        )
     with caplog.at_level(logging.WARNING):
         assert utils.validate_submission_id(123.0) == "123"
-        assert "Submission ID '123.0' contains decimals which are not supported" in caplog.text
+        assert (
+            "Submission ID '123.0' contains decimals which are not supported"
+            in caplog.text
+        )
 
 
 # TODO: Add a test for is_synapse_id_str(...)
diff --git a/tests/unit/synapseclient/unit_test_client.py b/tests/unit/synapseclient/unit_test_client.py
index b6374b729..c6dc01b9e 100644
--- a/tests/unit/synapseclient/unit_test_client.py
+++ b/tests/unit/synapseclient/unit_test_client.py
@@ -7,9 +7,9 @@
 import logging
 import os
 import tempfile
+import typing
 import urllib.request as urllib_request
 import uuid
-import typing
 from pathlib import Path
 from unittest.mock import ANY, AsyncMock, MagicMock, Mock, call, create_autospec, patch
 
@@ -2996,7 +2996,13 @@ def test_get_submission_with_annotations(syn: Synapse) -> None:
         assert evaluation_id == response["evaluationId"]
 
 
-def run_submission_test(syn: Synapse, submission_id: typing.Union[str, int], expected_id: str, should_warn: bool = False, caplog=None) -> None:
+def run_submission_test(
+    syn: Synapse,
+    submission_id: typing.Union[str, int],
+    expected_id: str,
+    should_warn: bool = False,
+    caplog=None,
+) -> None:
     """
     Common code for test_get_submission_valid_id and test_get_submission_invalid_id.
     Generates a dummy submission dictionary for regression testing, mocks the API calls,
@@ -3022,13 +3028,18 @@ def run_submission_test(syn: Synapse, submission_id: typing.Union[str, int], exp
         "entityBundleJSON": json.dumps({}),
     }
 
-    with patch.object(syn, "restGET") as restGET, patch.object(syn, "_getWithEntityBundle") as get_entity:
+    with patch.object(syn, "restGET") as restGET, patch.object(
+        syn, "_getWithEntityBundle"
+    ) as get_entity:
         restGET.return_value = submission
 
         if should_warn:
             with caplog.at_level(logging.WARNING):
                 syn.getSubmission(submission_id)
-                assert f"Submission ID '{submission_id}' contains decimals which are not supported" in caplog.text
+                assert (
+                    f"Submission ID '{submission_id}' contains decimals which are not supported"
+                    in caplog.text
+                )
         else:
             syn.getSubmission(submission_id)
 
@@ -3042,14 +3053,20 @@ def run_submission_test(syn: Synapse, submission_id: typing.Union[str, int], exp
 
 @pytest.mark.parametrize("submission_id, expected_id", [("123", "123"), (123, "123")])
 def test_get_submission_valid_id(syn: Synapse, submission_id, expected_id) -> None:
-    """ Test getSubmission with valid submission ID """
+    """Test getSubmission with valid submission ID"""
     run_submission_test(syn, submission_id, expected_id)
 
 
-@pytest.mark.parametrize("submission_id, expected_id", [("123.0", "123"), (123.0, "123")])
-def test_get_submission_invalid_id(syn: Synapse, submission_id, expected_id, caplog) -> None:
-    """ Test getSubmission with invalid submission ID """
-    run_submission_test(syn, submission_id, expected_id, should_warn=True, caplog=caplog)
+@pytest.mark.parametrize(
+    "submission_id, expected_id", [("123.0", "123"), (123.0, "123")]
+)
+def test_get_submission_invalid_id(
+    syn: Synapse, submission_id, expected_id, caplog
+) -> None:
+    """Test getSubmission with invalid submission ID"""
+    run_submission_test(
+        syn, submission_id, expected_id, should_warn=True, caplog=caplog
+    )
 
 
 class TestTableSnapshot:

From e7d392745fe01d933eb984b54ae72cce85f55944 Mon Sep 17 00:00:00 2001
From: Jenny Medina <medina.stsci@gmail.com>
Date: Fri, 27 Sep 2024 12:09:03 -0400
Subject: [PATCH 09/16] Update docstrings for getSubmission and
 getSubmissionStatus

---
 synapseclient/client.py | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/synapseclient/client.py b/synapseclient/client.py
index c38d72dcb..61bcf73c5 100644
--- a/synapseclient/client.py
+++ b/synapseclient/client.py
@@ -4812,10 +4812,11 @@ def getSubmission(
         self, id: typing.Union[str, int, collections.abc.Mapping], **kwargs
     ) -> Submission:
         """
-        Gets a [synapseclient.evaluation.Submission][] object by its id.
+        Gets a [synapseclient.evaluation.Submission][] object based on a given ID
+        or previous [synapseclient.evaluation.Submission][] object.
 
         Arguments:
-            id: The id of the submission to retrieve
+            id: The ID of the submission to retrieve or a [synapseclient.evaluation.Submission][] object
 
         Returns:
             A [synapseclient.evaluation.Submission][] object
@@ -4856,13 +4857,13 @@ def getSubmission(
         return submission
 
     def getSubmissionStatus(
-        self, submission: typing.Union[str, int]
+        self, submission: typing.Union[str, int, collections.abc.Mapping]
     ) -> SubmissionStatus:
         """
-        Downloads the status of a Submission.
+        Downloads the status of a Submission given its ID or previous [synapseclient.evaluation.Submission][] object.
 
         Arguments:
-            submission: The submission to lookup
+            submission: The submission to lookup (ID or [synapseclient.evaluation.Submission][] object)
 
         Returns:
             A [synapseclient.evaluation.SubmissionStatus][] object

From eda81d15a467b10c8a9556fe3c415f4c2c2d9c4d Mon Sep 17 00:00:00 2001
From: Jenny Medina <medina.stsci@gmail.com>
Date: Fri, 27 Sep 2024 15:37:34 -0400
Subject: [PATCH 10/16] Fix typo in validate_submission_id. Update
 unit_test_utils.py

---
 synapseclient/core/utils.py                   |  2 +-
 .../synapseclient/core/unit_test_utils.py     | 38 ++++++++++---------
 2 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/synapseclient/core/utils.py b/synapseclient/core/utils.py
index 8bbe4b2df..479113966 100644
--- a/synapseclient/core/utils.py
+++ b/synapseclient/core/utils.py
@@ -272,7 +272,7 @@ def validate_submission_id(
     elif isinstance(submission_id, collections.abc.Mapping):
         syn_id = _get_from_members_items_or_properties(submission_id, "id")
         if syn_id is not None:
-            return str((float(syn_id)))
+            return validate_submission_id(syn_id)
     else:
         int_submission_id = int(float(submission_id))
         LOGGER.warning(
diff --git a/tests/unit/synapseclient/core/unit_test_utils.py b/tests/unit/synapseclient/core/unit_test_utils.py
index 320e1d131..af52e30c1 100644
--- a/tests/unit/synapseclient/core/unit_test_utils.py
+++ b/tests/unit/synapseclient/core/unit_test_utils.py
@@ -101,25 +101,27 @@ def __init__(self, id_attr_name: str, id: str) -> None:
         assert utils.id_of(foo) == "123"
 
 
-def test_validate_submission_id(caplog) -> None:
-    # Test 1: Test valid inputs
-    assert utils.validate_submission_id("123") == "123"
-    assert utils.validate_submission_id(123) == "123"
-
-    # Test 2: Test invalid inputs get corrected
-    with caplog.at_level(logging.WARNING):
-        assert utils.validate_submission_id("123.0") == "123"
-        assert (
-            "Submission ID '123.0' contains decimals which are not supported"
-            in caplog.text
-        )
+@pytest.mark.parametrize(
+    "input_value, expected_output, expected_warning",
+    [
+        # Test 1: Valid inputs
+        ("123", "123", None),
+        (123, "123", None),
+        ({"id": "222"}, "222", None),
+
+        # Test 2: Invalid inputs that should be corrected
+        ("123.0", "123", "Submission ID '123.0' contains decimals which are not supported"),
+        (123.0, "123", "Submission ID '123.0' contains decimals which are not supported"),
+        ({"id": "999.222"}, "999", "Submission ID '999.222' contains decimals which are not supported"),
+    ]
+)
+def test_validate_submission_id(input_value, expected_output, expected_warning, caplog):
     with caplog.at_level(logging.WARNING):
-        assert utils.validate_submission_id(123.0) == "123"
-        assert (
-            "Submission ID '123.0' contains decimals which are not supported"
-            in caplog.text
-        )
-
+        assert utils.validate_submission_id(input_value) == expected_output
+        if expected_warning:
+            assert expected_warning in caplog.text
+        else:
+            assert not caplog.text
 
 # TODO: Add a test for is_synapse_id_str(...)
 # https://sagebionetworks.jira.com/browse/SYNPY-1425

From 1faa4b731a101404a1595bd296682a8777d84ce2 Mon Sep 17 00:00:00 2001
From: Jenny Medina <medina.stsci@gmail.com>
Date: Fri, 27 Sep 2024 15:52:21 -0400
Subject: [PATCH 11/16] Update unit_test_client.py

---
 tests/unit/synapseclient/unit_test_client.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/unit/synapseclient/unit_test_client.py b/tests/unit/synapseclient/unit_test_client.py
index c6dc01b9e..02611291b 100644
--- a/tests/unit/synapseclient/unit_test_client.py
+++ b/tests/unit/synapseclient/unit_test_client.py
@@ -3037,7 +3037,7 @@ def run_submission_test(
             with caplog.at_level(logging.WARNING):
                 syn.getSubmission(submission_id)
                 assert (
-                    f"Submission ID '{submission_id}' contains decimals which are not supported"
+                    f"contains decimals which are not supported"
                     in caplog.text
                 )
         else:
@@ -3051,14 +3051,14 @@ def run_submission_test(
         )
 
 
-@pytest.mark.parametrize("submission_id, expected_id", [("123", "123"), (123, "123")])
+@pytest.mark.parametrize("submission_id, expected_id", [("123", "123"), (123, "123"), ({"id": 123}, "123"), ({"id": "123"}, "123")])
 def test_get_submission_valid_id(syn: Synapse, submission_id, expected_id) -> None:
     """Test getSubmission with valid submission ID"""
     run_submission_test(syn, submission_id, expected_id)
 
 
 @pytest.mark.parametrize(
-    "submission_id, expected_id", [("123.0", "123"), (123.0, "123")]
+    "submission_id, expected_id", [("123.0", "123"), (123.0, "123"), ({"id": 123.0}, "123"), ({"id": "123.0"}, "123")]
 )
 def test_get_submission_invalid_id(
     syn: Synapse, submission_id, expected_id, caplog

From 09340005c4d2d4eb06897e0dae57bd1a6418f9c2 Mon Sep 17 00:00:00 2001
From: Jenny Medina <medina.stsci@gmail.com>
Date: Fri, 27 Sep 2024 15:54:47 -0400
Subject: [PATCH 12/16] Style

---
 .../synapseclient/core/unit_test_utils.py     | 22 ++++++++++++++-----
 tests/unit/synapseclient/unit_test_client.py  | 18 ++++++++++-----
 2 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/tests/unit/synapseclient/core/unit_test_utils.py b/tests/unit/synapseclient/core/unit_test_utils.py
index af52e30c1..568cf733e 100644
--- a/tests/unit/synapseclient/core/unit_test_utils.py
+++ b/tests/unit/synapseclient/core/unit_test_utils.py
@@ -108,12 +108,23 @@ def __init__(self, id_attr_name: str, id: str) -> None:
         ("123", "123", None),
         (123, "123", None),
         ({"id": "222"}, "222", None),
-
         # Test 2: Invalid inputs that should be corrected
-        ("123.0", "123", "Submission ID '123.0' contains decimals which are not supported"),
-        (123.0, "123", "Submission ID '123.0' contains decimals which are not supported"),
-        ({"id": "999.222"}, "999", "Submission ID '999.222' contains decimals which are not supported"),
-    ]
+        (
+            "123.0",
+            "123",
+            "Submission ID '123.0' contains decimals which are not supported",
+        ),
+        (
+            123.0,
+            "123",
+            "Submission ID '123.0' contains decimals which are not supported",
+        ),
+        (
+            {"id": "999.222"},
+            "999",
+            "Submission ID '999.222' contains decimals which are not supported",
+        ),
+    ],
 )
 def test_validate_submission_id(input_value, expected_output, expected_warning, caplog):
     with caplog.at_level(logging.WARNING):
@@ -123,6 +134,7 @@ def test_validate_submission_id(input_value, expected_output, expected_warning,
         else:
             assert not caplog.text
 
+
 # TODO: Add a test for is_synapse_id_str(...)
 # https://sagebionetworks.jira.com/browse/SYNPY-1425
 
diff --git a/tests/unit/synapseclient/unit_test_client.py b/tests/unit/synapseclient/unit_test_client.py
index 02611291b..37405283f 100644
--- a/tests/unit/synapseclient/unit_test_client.py
+++ b/tests/unit/synapseclient/unit_test_client.py
@@ -3036,10 +3036,7 @@ def run_submission_test(
         if should_warn:
             with caplog.at_level(logging.WARNING):
                 syn.getSubmission(submission_id)
-                assert (
-                    f"contains decimals which are not supported"
-                    in caplog.text
-                )
+                assert f"contains decimals which are not supported" in caplog.text
         else:
             syn.getSubmission(submission_id)
 
@@ -3051,14 +3048,23 @@ def run_submission_test(
         )
 
 
-@pytest.mark.parametrize("submission_id, expected_id", [("123", "123"), (123, "123"), ({"id": 123}, "123"), ({"id": "123"}, "123")])
+@pytest.mark.parametrize(
+    "submission_id, expected_id",
+    [("123", "123"), (123, "123"), ({"id": 123}, "123"), ({"id": "123"}, "123")],
+)
 def test_get_submission_valid_id(syn: Synapse, submission_id, expected_id) -> None:
     """Test getSubmission with valid submission ID"""
     run_submission_test(syn, submission_id, expected_id)
 
 
 @pytest.mark.parametrize(
-    "submission_id, expected_id", [("123.0", "123"), (123.0, "123"), ({"id": 123.0}, "123"), ({"id": "123.0"}, "123")]
+    "submission_id, expected_id",
+    [
+        ("123.0", "123"),
+        (123.0, "123"),
+        ({"id": 123.0}, "123"),
+        ({"id": "123.0"}, "123"),
+    ],
 )
 def test_get_submission_invalid_id(
     syn: Synapse, submission_id, expected_id, caplog

From e857a12555dbd641ef1a2de5eb3ebd0976866eb0 Mon Sep 17 00:00:00 2001
From: Jenny Medina <medina.stsci@gmail.com>
Date: Fri, 27 Sep 2024 16:21:24 -0400
Subject: [PATCH 13/16] rename

---
 tests/unit/synapseclient/unit_test_client.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/unit/synapseclient/unit_test_client.py b/tests/unit/synapseclient/unit_test_client.py
index 37405283f..bb53f1f6b 100644
--- a/tests/unit/synapseclient/unit_test_client.py
+++ b/tests/unit/synapseclient/unit_test_client.py
@@ -2996,7 +2996,7 @@ def test_get_submission_with_annotations(syn: Synapse) -> None:
         assert evaluation_id == response["evaluationId"]
 
 
-def run_submission_test(
+def run_get_submission_test(
     syn: Synapse,
     submission_id: typing.Union[str, int],
     expected_id: str,
@@ -3054,7 +3054,7 @@ def run_submission_test(
 )
 def test_get_submission_valid_id(syn: Synapse, submission_id, expected_id) -> None:
     """Test getSubmission with valid submission ID"""
-    run_submission_test(syn, submission_id, expected_id)
+    run_get_submission_test(syn, submission_id, expected_id)
 
 
 @pytest.mark.parametrize(
@@ -3070,7 +3070,7 @@ def test_get_submission_invalid_id(
     syn: Synapse, submission_id, expected_id, caplog
 ) -> None:
     """Test getSubmission with invalid submission ID"""
-    run_submission_test(
+    run_get_submission_test(
         syn, submission_id, expected_id, should_warn=True, caplog=caplog
     )
 

From cdeab2061b25387f3a0260f516771b7e16af6068 Mon Sep 17 00:00:00 2001
From: Jenny Medina <medina.stsci@gmail.com>
Date: Mon, 30 Sep 2024 12:29:57 -0400
Subject: [PATCH 14/16] Test that getSubmission and getSubmissionStatus can
 work together

---
 tests/unit/synapseclient/unit_test_client.py | 72 ++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/tests/unit/synapseclient/unit_test_client.py b/tests/unit/synapseclient/unit_test_client.py
index bb53f1f6b..e7810efbe 100644
--- a/tests/unit/synapseclient/unit_test_client.py
+++ b/tests/unit/synapseclient/unit_test_client.py
@@ -60,6 +60,7 @@
 )
 from synapseclient.core.models.dict_object import DictObject
 from synapseclient.core.upload import upload_functions
+from synapseclient.evaluation import Submission, SubmissionStatus
 
 GET_FILE_HANDLE_FOR_DOWNLOAD = (
     "synapseclient.core.download.download_functions.get_file_handle_for_download_async"
@@ -3075,6 +3076,77 @@ def test_get_submission_invalid_id(
     )
 
 
+def test_get_submission_and_submission_status_interchangeability(
+    syn: Synapse, caplog
+) -> None:
+    """Test interchangeability of getSubmission and getSubmissionStatus."""
+
+    # Establish some dummy variables to work with
+    evaluation_id = 98765
+    submission_id = 9745366.0
+    expected_submission_id = "9745366"
+
+    # Establish an expected return for `getSubmissionStatus`
+    submission_status_return = {
+        "id": expected_submission_id,
+        "etag": "000",
+        "status": "RECEIVED",
+    }
+
+    # Establish an expected return for `getSubmission`
+    submission_return = {
+        "id": expected_submission_id,
+        "evaluationId": evaluation_id,
+        "entityId": expected_submission_id,
+        "versionNumber": 1,
+        "entityBundleJSON": json.dumps({}),
+    }
+
+    # Let's mock all the API calls made within these two methods
+    with patch.object(syn, "restGET") as restGET, patch.object(
+        Submission, "getURI"
+    ) as get_submission_uri, patch.object(
+        SubmissionStatus, "getURI"
+    ) as get_status_uri, patch.object(
+        syn, "_getWithEntityBundle"
+    ):
+        get_submission_uri.return_value = (
+            f"/evaluation/submission/{expected_submission_id}"
+        )
+        get_status_uri.return_value = (
+            f"/evaluation/submission/{expected_submission_id}/status"
+        )
+
+        # Establish a return for all the calls to restGET we will be making in this test
+        restGET.side_effect = [
+            # Step 1 call to `getSubmission`
+            submission_return,
+            # Step 2 call to `getSubmissionStatus`
+            submission_status_return,
+        ]
+
+        # Step 1: Call `getSubmission` with float ID
+        restGET.return_value = submission_return
+        submission_result = syn.getSubmission(submission_id)
+
+        # Step 2: Call `getSubmissionStatus` with the `Submission` object from above
+        restGET.reset_mock()
+        restGET.return_value = submission_status_return
+        submission_status_result = syn.getSubmissionStatus(submission_result)
+
+        # Validate that getSubmission and getSubmissionStatus are called with correct URIs
+        # in `getURI` calls
+        get_submission_uri.assert_called_once_with(expected_submission_id)
+        get_status_uri.assert_called_once_with(expected_submission_id)
+
+        # Validate final output is as expected
+        assert (
+            submission_result["id"]
+            == submission_status_result["id"]
+            == expected_submission_id
+        )
+
+
 class TestTableSnapshot:
     def test__create_table_snapshot(self, syn: Synapse) -> None:
         """Testing creating table snapshots"""

From c5ae645061e33ebd5cfafdd316b89c63854149e9 Mon Sep 17 00:00:00 2001
From: Jenny Medina <medina.stsci@gmail.com>
Date: Mon, 30 Sep 2024 12:44:12 -0400
Subject: [PATCH 15/16] Catch for non-digit values and add unit test

---
 synapseclient/core/utils.py                      | 7 ++++++-
 tests/unit/synapseclient/core/unit_test_utils.py | 9 +++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/synapseclient/core/utils.py b/synapseclient/core/utils.py
index 479113966..6a3de5a39 100644
--- a/synapseclient/core/utils.py
+++ b/synapseclient/core/utils.py
@@ -274,7 +274,12 @@ def validate_submission_id(
         if syn_id is not None:
             return validate_submission_id(syn_id)
     else:
-        int_submission_id = int(float(submission_id))
+        try:
+            int_submission_id = int(float(submission_id))
+        except ValueError:
+            raise ValueError(
+                f"Submission ID '{submission_id}' is not a valid submission ID. Please use digits only."
+            )
         LOGGER.warning(
             f"Submission ID '{submission_id}' contains decimals which are not supported. "
             f"Submission ID will be converted to '{int_submission_id}'."
diff --git a/tests/unit/synapseclient/core/unit_test_utils.py b/tests/unit/synapseclient/core/unit_test_utils.py
index 568cf733e..30478f7fe 100644
--- a/tests/unit/synapseclient/core/unit_test_utils.py
+++ b/tests/unit/synapseclient/core/unit_test_utils.py
@@ -135,6 +135,15 @@ def test_validate_submission_id(input_value, expected_output, expected_warning,
             assert not caplog.text
 
 
+def test_validate_submission_id_letters_input() -> None:
+    letters_input = "syn123"
+    expected_error = f"Submission ID '{letters_input}' is not a valid submission ID. Please use digits only."
+    with pytest.raises(ValueError) as err:
+        utils.validate_submission_id(letters_input)
+
+    assert str(err.value) == expected_error
+
+
 # TODO: Add a test for is_synapse_id_str(...)
 # https://sagebionetworks.jira.com/browse/SYNPY-1425
 

From 5fb25ca02cd2e4892eb838d94ff934aa789937b8 Mon Sep 17 00:00:00 2001
From: Jenny Medina <medina.stsci@gmail.com>
Date: Tue, 1 Oct 2024 09:47:02 -0400
Subject: [PATCH 16/16] Remove logic to handle native python warnings

---
 synapseclient/core/utils.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/synapseclient/core/utils.py b/synapseclient/core/utils.py
index 6a3de5a39..d1cefaa0b 100644
--- a/synapseclient/core/utils.py
+++ b/synapseclient/core/utils.py
@@ -53,7 +53,6 @@
 # Set up logging
 LOGGER_NAME = DEFAULT_LOGGER_NAME
 LOGGER = logging.getLogger(LOGGER_NAME)
-logging.getLogger("py.warnings").handlers = LOGGER.handlers
 
 
 def md5_for_file(