From bc1cdf910245965a1398503fd89227e744ad9709 Mon Sep 17 00:00:00 2001
From: Matthew Richards <matthew.richards@stfc.ac.uk>
Date: Thu, 30 Jul 2020 14:29:07 +0000
Subject: [PATCH 1/4]  #136: Make improvements as per PR for this issue

---
 common/backend.py                             | 11 +++--
 common/database_backend.py                    | 12 ++---
 common/database_helpers.py                    | 34 +++++++-------
 common/python_icat_backend.py                 | 12 ++---
 common/python_icat_helpers.py                 | 47 ++++++++++---------
 src/main.py                                   |  2 +-
 src/resources/entities/entity_endpoint.py     | 14 +++---
 .../table_endpoints/table_endpoints.py        |  8 ++--
 8 files changed, 71 insertions(+), 69 deletions(-)

diff --git a/common/backend.py b/common/backend.py
index dbe31edd..7704ed08 100644
--- a/common/backend.py
+++ b/common/backend.py
@@ -97,32 +97,33 @@ def count_with_filters(self, session_id, entity_type, filters):
         pass
 
     @abstractmethod
-    def get_with_id(self, session_id, entity_type, id):
+    def get_with_id(self, session_id, entity_type, id_):
         """
         Gets the entity matching the given ID for the given entity type
         :param session_id: The session id of the requesting user
         :param entity_type: The type of entity
-        :param id: the id of the record to find
+        :param id_: the id of the record to find
         :return: the entity retrieved
         """
         pass
 
     @abstractmethod
-    def delete_with_id(self, session_id, entity_type, id):
+    def delete_with_id(self, session_id, entity_type, id_):
         """
         Deletes the row matching the given ID for the given entity type
         :param session_id: The session id of the requesting user
         :param table: the table to be searched
-        :param id: the id of the record to delete
+        :param id_: the id of the record to delete
         """
         pass
 
     @abstractmethod
-    def update_with_id(self, session_id, entity_type, id, data):
+    def update_with_id(self, session_id, entity_type, id_, data):
         """
         Updates the row matching the given ID for the given entity type
         :param session_id: The session id of the requesting user
         :param entity_type: The type of entity
+        :param id_: the id of the record to update
         :param data: The dictionary that the entity should be updated with
         :return: The updated entity.
         """
diff --git a/common/database_backend.py b/common/database_backend.py
index 1ab207e1..ec5e2c5f 100644
--- a/common/database_backend.py
+++ b/common/database_backend.py
@@ -67,18 +67,18 @@ def count_with_filters(self, session_id, table, filters):
 
     @requires_session_id
     @queries_records
-    def get_with_id(self, session_id, table, id):
-        return get_row_by_id(table, id).to_dict()
+    def get_with_id(self, session_id, table, id_):
+        return get_row_by_id(table, id_).to_dict()
 
     @requires_session_id
     @queries_records
-    def delete_with_id(self, session_id, table, id):
-        return delete_row_by_id(table, id)
+    def delete_with_id(self, session_id, table, id_):
+        return delete_row_by_id(table, id_)
 
     @requires_session_id
     @queries_records
-    def update_with_id(self, session_id, table, id, data):
-        return update_row_from_id(table, id, data)
+    def update_with_id(self, session_id, table, id_, data):
+        return update_row_from_id(table, id_, data)
 
     @requires_session_id
     @queries_records
diff --git a/common/database_helpers.py b/common/database_helpers.py
index b3d94d8a..2f7c7f8f 100644
--- a/common/database_helpers.py
+++ b/common/database_helpers.py
@@ -68,7 +68,7 @@ def commit_changes(self):
         """
         Commits all changes to the database and closes the session
         """
-        log.info(f" Commiting changes to {self.table}")
+        log.info(" Committing changes to %s", self.table)
         self.session.commit()
 
 
@@ -146,7 +146,7 @@ def __init__(self, table, row, new_values):
         self.new_values = new_values
 
     def execute_query(self):
-        log.info(f" Updating row in {self.table}")
+        log.info(" Updating row in %s", self.table)
         self.row.update_from_dict(self.new_values)
         self.session.add(self.row)
         self.commit_changes()
@@ -159,7 +159,7 @@ def __init__(self, table, row):
         self.row = row
 
     def execute_query(self):
-        log.info(f" Deleting row {self.row} from {self.table.__tablename__}")
+        log.info(" Deleting row %s from %s", self.row, self.table.__tablename__)
         self.session.delete(self.row)
         self.commit_changes()
 
@@ -394,40 +394,40 @@ def create_rows_from_json(table, data):
     return create_row_from_json(table, data)
 
 
-def get_row_by_id(table, id):
+def get_row_by_id(table, id_):
     """
     Gets the row matching the given ID from the given table, raises MissingRecordError if it can not be found
     :param table: the table to be searched
-    :param id: the id of the record to find
+    :param id_: the id of the record to find
     :return: the record retrieved
     """
     with ReadQuery(table) as read_query:
-        log.info(f" Querying {table.__tablename__} for record with ID: {id}")
-        where_filter = WhereFilter("ID", id, "eq")
+        log.info(" Querying %s for record with ID: %d", table.__tablename__, id_)
+        where_filter = WhereFilter("ID", id_, "eq")
         where_filter.apply_filter(read_query)
         return read_query.get_single_result()
 
 
-def delete_row_by_id(table, id):
+def delete_row_by_id(table, id_):
     """
     Deletes the row matching the given ID from the given table, raises MissingRecordError if it can not be found
     :param table: the table to be searched
-    :param id: the id of the record to delete
+    :param id_: the id of the record to delete
     """
-    log.info(f" Deleting row from {table.__tablename__} with ID: {id}")
-    row = get_row_by_id(table, id)
+    log.info(" Deleting row from %s with ID: %d", table.__tablename__, id_)
+    row = get_row_by_id(table, id_)
     with DeleteQuery(table, row) as delete_query:
         delete_query.execute_query()
 
 
-def update_row_from_id(table, id, new_values):
+def update_row_from_id(table, id_, new_values):
     """
     Updates a record in a table
     :param table: The table the record is in
-    :param id: The id of the record
+    :param id_: The id of the record
     :param new_values: A JSON string containing what columns are to be updated
     """
-    row = get_row_by_id(table, id)
+    row = get_row_by_id(table, id_)
     with UpdateQuery(table, row, new_values) as update_query:
         update_query.execute_query()
 
@@ -496,7 +496,7 @@ def get_first_filtered_row(table, filters):
     :param filters: the filter to be applied to the query
     :return: the first row matching the filter
     """
-    log.info(f" Getting first filtered row for {table.__tablename__}")
+    log.info(" Getting first filtered row for %s", table.__tablename__)
     return get_rows_by_filter(table, filters)[0]
 
 
@@ -508,7 +508,7 @@ def get_filtered_row_count(table, filters):
     :return: int: the count of the rows
     """
 
-    log.info(f" getting count for {table.__tablename__}")
+    log.info(" getting count for %s", table.__tablename__)
     with CountQuery(table) as count_query:
         filter_handler = FilterOrderHandler()
         filter_handler.add_filters(filters)
@@ -523,7 +523,7 @@ def patch_entities(table, json_list):
     :param json_list: the list of updated values or a dictionary
     :return: The list of updated rows.
     """
-    log.info(f" Patching entities in {table.__tablename__}")
+    log.info(" Patching entities in %s", table.__tablename__)
     results = []
     if type(json_list) is dict:
         for key in json_list:
diff --git a/common/python_icat_backend.py b/common/python_icat_backend.py
index 6c4ca76b..903b5297 100644
--- a/common/python_icat_backend.py
+++ b/common/python_icat_backend.py
@@ -80,18 +80,18 @@ def count_with_filters(self, session_id, table, filters):
 
     @requires_session_id
     @queries_records
-    def get_with_id(self, session_id, table, id):
-        return get_entity_by_id(self.client, table.__name__, id, True)
+    def get_with_id(self, session_id, table, id_):
+        return get_entity_by_id(self.client, table.__name__, id_, True)
 
     @requires_session_id
     @queries_records
-    def delete_with_id(self, session_id, table, id):
-        return delete_entity_by_id(self.client, table.__name__, id)
+    def delete_with_id(self, session_id, table, id_):
+        return delete_entity_by_id(self.client, table.__name__, id_)
 
     @requires_session_id
     @queries_records
-    def update_with_id(self, session_id, table, id, data):
-        return update_entity_by_id(self.client, table.__name__, id, data)
+    def update_with_id(self, session_id, table, id_, data):
+        return update_entity_by_id(self.client, table.__name__, id_, data)
 
     @requires_session_id
     @queries_records
diff --git a/common/python_icat_helpers.py b/common/python_icat_helpers.py
index 23aa8009..8104a2b2 100644
--- a/common/python_icat_helpers.py
+++ b/common/python_icat_helpers.py
@@ -25,7 +25,7 @@ def wrapper_requires_session(*args, **kwargs):
             client = args[0].client
             # Find out if session has expired
             session_time = client.getRemainingMinutes()
-            log.info(f"Session time: {session_time}")
+            log.info("Session time: %d", session_time)
             if session_time < 0:
                 raise AuthenticationError("Forbidden")
             else:
@@ -127,8 +127,9 @@ def execute_icat_query(client, query, return_json_formattable=False):
             dict_result = result.as_dict()
             for key, value in dict_result.items():
                 # Convert datetime objects to strings so they can be JSON serialisable
-                if isinstance(dict_result[key], datetime):
-                    dict_result[key] = str(dict_result[key])
+                if isinstance(value, datetime):
+                    # Remove timezone data which isn't utilised in ICAT and convert to ISO format
+                    dict_result[key] = value.replace(tzinfo=None).isoformat(' ')
 
             data.append(dict_result)
         return data
@@ -206,8 +207,8 @@ def str_to_date_object(icat_attribute, data):
         `accepted_date_format`
     """
 
-    log.debug(f"ICAT Attribute: {icat_attribute}, Type: {type(icat_attribute)}")
-    log.debug(f"Data: {data}, Type: {type(data)}")
+    log.debug("ICAT Attribute: %s, Type: %s", icat_attribute, type(icat_attribute))
+    log.debug("Data: %s, Type: %s", data, type(data))
 
     accepted_date_format = "%Y-%m-%d %H:%M:%S"
 
@@ -246,7 +247,7 @@ def update_attributes(object, dictionary):
     object.update()
 
 
-def get_entity_by_id(client, table_name, id, return_json_formattable_data):
+def get_entity_by_id(client, table_name, id_, return_json_formattable_data):
     """
     Gets a record of a given ID from the specified entity
 
@@ -254,8 +255,8 @@ def get_entity_by_id(client, table_name, id, return_json_formattable_data):
     :type client: :class:`icat.client.Client`
     :param table_name: Table name to extract which entity to use
     :type table_name: :class:`str`
-    :param id: ID number of the entity to retrieve
-    :type id: :class:`int`
+    :param id_: ID number of the entity to retrieve
+    :type id_: :class:`int`
     :param return_json_formattable_data: Flag to determine whether the data should be returned as a
         list of data ready to be converted straight to JSON (i.e. if the data will be used as a
         response for an API call) or whether to leave the data in a Python ICAT format
@@ -265,21 +266,21 @@ def get_entity_by_id(client, table_name, id, return_json_formattable_data):
     """
 
     # Set query condition for the selected ID
-    id_condition = create_condition('id', '=', id)
+    id_condition = create_condition('id', '=', id_)
 
     selected_entity_name = get_python_icat_entity_name(client, table_name)
 
     id_query = construct_icat_query(client, selected_entity_name, conditions=id_condition, includes="1")
     entity_by_id_data = execute_icat_query(client, id_query, return_json_formattable_data)
 
-    if entity_by_id_data == []:
+    if not entity_by_id_data:
         # Cannot find any data matching the given ID
         raise MissingRecordError("No result found")
+    else: 
+        return entity_by_id_data[0]
 
-    return entity_by_id_data
 
-
-def delete_entity_by_id(client, table_name, id):
+def delete_entity_by_id(client, table_name, id_):
     """
     Deletes a record of a given ID of the specified entity
 
@@ -287,15 +288,15 @@ def delete_entity_by_id(client, table_name, id):
     :type client: :class:`icat.client.Client`
     :param table_name: Table name to extract which entity to delete
     :type table_name: :class:`str`
-    :param id: ID number of the entity to delete
-    :type id: :class:`int`
+    :param id_: ID number of the entity to delete
+    :type id_: :class:`int`
     """
 
-    entity_id_data = get_entity_by_id(client, table_name, id, False)
-    client.delete(entity_id_data[0])
+    entity_id_data = get_entity_by_id(client, table_name, id_, False)
+    client.delete(entity_id_data)
 
 
-def update_entity_by_id(client, table_name, id, new_data):
+def update_entity_by_id(client, table_name, id_, new_data):
     """
     Gets a record of a given ID of the specified entity
 
@@ -303,19 +304,19 @@ def update_entity_by_id(client, table_name, id, new_data):
     :type client: :class:`icat.client.Client`
     :param table_name: Table name to extract which entity to use
     :type table_name: :class:`str`
-    :param id: ID number of the entity to retrieve
-    :type id: :class:`int`
+    :param id_: ID number of the entity to retrieve
+    :type id_: :class:`int`
     :param new_data: JSON from request body providing new data to update the record with the
         specified ID
     :return: The updated record of the specified ID from the given entity
     """
 
-    entity_id_data = get_entity_by_id(client, table_name, id, False)
+    entity_id_data = get_entity_by_id(client, table_name, id_, False)
     # There will only ever be one record associated with a single ID - if a record with the
     # specified ID cannot be found, it'll be picked up by the MissingRecordError in 
     # get_entity_by_id()
-    update_attributes(entity_id_data[0], new_data)
+    update_attributes(entity_id_data, new_data)
 
     # The record is re-obtained from Python ICAT (rather than using entity_id_data) to show to the
     # user whether the change has actually been applied
-    return get_entity_by_id(client, table_name, id, True)
+    return get_entity_by_id(client, table_name, id_, True)
diff --git a/src/main.py b/src/main.py
index e1d90cc8..8ffa6699 100644
--- a/src/main.py
+++ b/src/main.py
@@ -57,7 +57,7 @@ def handle_error(e):
     get_id_endpoint_resource = get_id_endpoint(
         entity_name, endpoints[entity_name])
     api.add_resource(get_id_endpoint_resource,
-                     f"/{entity_name.lower()}/<int:id>")
+                     f"/{entity_name.lower()}/<int:id_>")
     spec.path(resource=get_id_endpoint_resource, api=api)
 
     get_count_endpoint_resource = get_count_endpoint(
diff --git a/src/resources/entities/entity_endpoint.py b/src/resources/entities/entity_endpoint.py
index 4779f929..126e9db9 100644
--- a/src/resources/entities/entity_endpoint.py
+++ b/src/resources/entities/entity_endpoint.py
@@ -149,8 +149,8 @@ def get_id_endpoint(name, table):
     """
     class EndpointWithID(Resource):
 
-        def get(self, id):
-            return backend.get_with_id(get_session_id_from_auth_header(), table, id), 200
+        def get(self, id_):
+            return backend.get_with_id(get_session_id_from_auth_header(), table, id_), 200
 
         get.__doc__ = f"""
             ---
@@ -182,9 +182,9 @@ def get(self, id):
                     description: No such record - Unable to find a record in the database
             """
 
-        def delete(self, id):
+        def delete(self, id_):
             backend.delete_with_id(
-                get_session_id_from_auth_header(), table, id)
+                get_session_id_from_auth_header(), table, id_)
             return "", 204
 
         delete.__doc__ = f"""
@@ -213,10 +213,10 @@ def delete(self, id):
                     description: No such record - Unable to find a record in the database
             """
 
-        def patch(self, id):
+        def patch(self, id_):
             session_id = get_session_id_from_auth_header()
-            backend.update_with_id(session_id, table, id, request.json)
-            return backend.get_with_id(session_id, table, id), 200
+            backend.update_with_id(session_id, table, id_, request.json)
+            return backend.get_with_id(session_id, table, id_), 200
 
         patch.__doc__ = f"""
             ---
diff --git a/src/resources/table_endpoints/table_endpoints.py b/src/resources/table_endpoints/table_endpoints.py
index 31f77991..70121ec7 100644
--- a/src/resources/table_endpoints/table_endpoints.py
+++ b/src/resources/table_endpoints/table_endpoints.py
@@ -7,7 +7,7 @@
 
 
 class InstrumentsFacilityCycles(Resource):
-    def get(self, id):
+    def get(self, id_):
         """
         ---
         summary: Get an Instrument's FacilityCycles 
@@ -45,11 +45,11 @@ def get(self, id):
             404:
                 description: No such record - Unable to find a record in the database
         """
-        return backend.get_facility_cycles_for_instrument(get_session_id_from_auth_header(), id, get_filters_from_query_string()), 200
+        return backend.get_facility_cycles_for_instrument(get_session_id_from_auth_header(), id_, get_filters_from_query_string()), 200
 
 
 class InstrumentsFacilityCyclesCount(Resource):
-    def get(self, id):
+    def get(self, id_):
         """
         ---
         summary: Count an Instrument's FacilityCycles 
@@ -81,7 +81,7 @@ def get(self, id):
             404:
                 description: No such record - Unable to find a record in the database
         """
-        return backend.get_facility_cycles_for_instrument_count(get_session_id_from_auth_header(), id, get_filters_from_query_string()), 200
+        return backend.get_facility_cycles_for_instrument_count(get_session_id_from_auth_header(), id_, get_filters_from_query_string()), 200
 
 
 class InstrumentsFacilityCyclesInvestigations(Resource):

From f6ac645272049a2f5876814c66680f00cbc1c3c0 Mon Sep 17 00:00:00 2001
From: Matthew Richards <matthew.richards@stfc.ac.uk>
Date: Thu, 30 Jul 2020 14:47:32 +0000
Subject: [PATCH 2/4]  #137: Make improvements as per PR for this issue

---
 common/python_icat_helpers.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/common/python_icat_helpers.py b/common/python_icat_helpers.py
index d654597c..74a36090 100644
--- a/common/python_icat_helpers.py
+++ b/common/python_icat_helpers.py
@@ -329,7 +329,7 @@ def get_entity_with_filters(client, table_name, filters):
     query = construct_icat_query(client, selected_entity_name)
     data = execute_icat_query(client, query, True)
 
-    if data == []:
+    if not data:
         raise MissingRecordError("No results found")
-
-    return data
+    else:
+        return data

From 9611a1143b4e27b441d676db342b20b55d84c469 Mon Sep 17 00:00:00 2001
From: Matthew Richards <matthew.richards@stfc.ac.uk>
Date: Mon, 10 Aug 2020 12:23:26 +0000
Subject: [PATCH 3/4]  #136: Changes made as requested in pull request

---
 common/python_icat_helpers.py | 43 +++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/common/python_icat_helpers.py b/common/python_icat_helpers.py
index 8104a2b2..5337fbfb 100644
--- a/common/python_icat_helpers.py
+++ b/common/python_icat_helpers.py
@@ -193,11 +193,16 @@ def create_condition(attribute_name, operator, value):
     return condition
 
 
-def str_to_date_object(icat_attribute, data):
+def str_to_datetime_object(icat_attribute, data):
     """
     Where data is stored as dates in ICAT (which this function determines), convert strings (i.e.
     user data from PATCH/POST requests) into datetime objects so they can be stored in ICAT
 
+    Python 3.7+ has support for `datetime.fromisoformat()` which would be a more elegant solution
+    to this conversion operation since dates are converted into ISO format within this file,
+    however, the production instance of this API is typically built on Python 3.6, and it doesn't
+    seem of enough value to mandate 3.7 for a single line of code
+
     :param icat_attribute: Attribute that will be updated with new data
     :type icat_attribute: Any valid data type that can be stored in Python ICAT
     :param data: Single data value from the request body
@@ -212,39 +217,39 @@ def str_to_date_object(icat_attribute, data):
 
     accepted_date_format = "%Y-%m-%d %H:%M:%S"
 
-    if isinstance(icat_attribute, datetime):
-        try:
-            data = datetime.strptime(data, accepted_date_format)
-        except ValueError:
-            raise BadRequestError(f"Bad request made, the date entered is not in the correct format. Use the {accepted_date_format} format to submit dates to the API")
+    try:
+        data = datetime.strptime(data, accepted_date_format)
+    except ValueError:
+        raise BadRequestError(f"Bad request made, the date entered is not in the correct format. Use the {accepted_date_format} format to submit dates to the API")
 
     return data
 
 
-def update_attributes(object, dictionary):
+def update_attributes(old_entity, new_entity):
     """
     Updates the attribute(s) of a given object which is a record of an entity from Python ICAT
 
-    :param object: An entity from Python ICAT
+    :param old_entity: An existing entity record from Python ICAT
     :type object: :class:`icat.entities.ENTITY`
-    :param dictionary: Dictionary containing the new data to be modified
-    :type dictionary: :class:`dict`
-    :raises BadRequestError: If the attribute cannot be edited - typically if Python ICAT doesn't
-        allow an attribute to be edited (e.g. modId & modTime)
+    :param new_entity: Dictionary containing the new data to be modified
+    :type new_entity: :class:`dict`
+    :raises BadRequestError: If the attribute cannot be found, or if it cannot be edited -
+        typically if Python ICAT doesn't allow an attribute to be edited (e.g. modId & modTime)
     """
-    for key in dictionary:
+    for key in new_entity:
         try:
-            original_data_attribute = getattr(object, key)
-            dictionary[key] = str_to_date_object(original_data_attribute, dictionary[key])
+            original_data_attribute = getattr(old_entity, key)
+            if isinstance(original_data_attribute, datetime):
+                new_entity[key] = str_to_datetime_object(original_data_attribute, new_entity[key])
         except AttributeError:
-            raise BadRequestError(f"Bad request made, cannot find attribute '{key}' within the {object.BeanName} entity")
+            raise BadRequestError(f"Bad request made, cannot find attribute '{key}' within the {old_entity.BeanName} entity")
 
         try:
-            setattr(object, key, dictionary[key])
+            setattr(old_entity, key, new_entity[key])
         except AttributeError:
-            raise BadRequestError(f"Bad request made, cannot modify attribute '{key}' within the {object.BeanName} entity")
+            raise BadRequestError(f"Bad request made, cannot modify attribute '{key}' within the {old_entity.BeanName} entity")
 
-    object.update()
+    old_entity.update()
 
 
 def get_entity_by_id(client, table_name, id_, return_json_formattable_data):

From 511655e7fa652c53760b0f888ebd7aba4c751749 Mon Sep 17 00:00:00 2001
From: Matthew Richards <matthew.richards@stfc.ac.uk>
Date: Tue, 11 Aug 2020 10:21:49 +0000
Subject: [PATCH 4/4]  #136: Allow round-tripping of datetime data

- Added an ICAT exception where data modification was stopped to prevent icatdb from being put into an invalid state
---
 common/constants.py           |  1 +
 common/python_icat_helpers.py | 24 ++++++++++++------------
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/common/constants.py b/common/constants.py
index c831f7e7..9d3fef36 100644
--- a/common/constants.py
+++ b/common/constants.py
@@ -3,3 +3,4 @@
 
 class Constants:
     DATABASE_URL = config.get_db_url()
+    ACCEPTED_DATE_FORMAT = "%Y-%m-%d %H:%M:%S"
diff --git a/common/python_icat_helpers.py b/common/python_icat_helpers.py
index 5337fbfb..77059fb8 100644
--- a/common/python_icat_helpers.py
+++ b/common/python_icat_helpers.py
@@ -3,8 +3,10 @@
 from datetime import datetime, timedelta
 
 from icat.query import Query
-from icat.exception import ICATSessionError
+from icat.exception import ICATSessionError, ICATValidationError
 from common.exceptions import AuthenticationError, BadRequestError, MissingRecordError, PythonICATError
+from common.constants import Constants
+
 
 log = logging.getLogger()
 
@@ -128,8 +130,8 @@ def execute_icat_query(client, query, return_json_formattable=False):
             for key, value in dict_result.items():
                 # Convert datetime objects to strings so they can be JSON serialisable
                 if isinstance(value, datetime):
-                    # Remove timezone data which isn't utilised in ICAT and convert to ISO format
-                    dict_result[key] = value.replace(tzinfo=None).isoformat(' ')
+                    # Remove timezone data which isn't utilised in ICAT
+                    dict_result[key] = value.replace(tzinfo=None).strftime(Constants.ACCEPTED_DATE_FORMAT)
 
             data.append(dict_result)
         return data
@@ -209,18 +211,13 @@ def str_to_datetime_object(icat_attribute, data):
     :type data: Data type of the data as per user's request body
     :return: Date converted into a :class:`datetime` object
     :raises BadRequestError: If the date is entered in the incorrect format, as per
-        `accepted_date_format`
+        `Constants.ACCEPTED_DATE_FORMAT`
     """
 
-    log.debug("ICAT Attribute: %s, Type: %s", icat_attribute, type(icat_attribute))
-    log.debug("Data: %s, Type: %s", data, type(data))
-
-    accepted_date_format = "%Y-%m-%d %H:%M:%S"
-
     try:
-        data = datetime.strptime(data, accepted_date_format)
+        data = datetime.strptime(data, Constants.ACCEPTED_DATE_FORMAT)
     except ValueError:
-        raise BadRequestError(f"Bad request made, the date entered is not in the correct format. Use the {accepted_date_format} format to submit dates to the API")
+        raise BadRequestError(f"Bad request made, the date entered is not in the correct format. Use the {Constants.ACCEPTED_DATE_FORMAT} format to submit dates to the API")
 
     return data
 
@@ -249,7 +246,10 @@ def update_attributes(old_entity, new_entity):
         except AttributeError:
             raise BadRequestError(f"Bad request made, cannot modify attribute '{key}' within the {old_entity.BeanName} entity")
 
-    old_entity.update()
+    try:
+        old_entity.update()
+    except ICATValidationError as e:
+        raise PythonICATError(e)
 
 
 def get_entity_by_id(client, table_name, id_, return_json_formattable_data):