Skip to content

Commit

Permalink
Added: improved error handling for update api route (#42)
Browse files Browse the repository at this point in the history
  • Loading branch information
signebedi committed Mar 24, 2024
1 parent 20dcf95 commit 85fbafa
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 17 deletions.
32 changes: 24 additions & 8 deletions libreforms_fastapi/app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@
ManageTinyDB,
ManageMongoDB,
CollectionDoesNotExist,
DocumentDoesNotExist,
DocumentIsDeleted,
InsufficientPermissions
)

from libreforms_fastapi.utils.pydantic_models import (
Expand Down Expand Up @@ -585,7 +588,7 @@ async def api_form_update(form_name: str, document_id: str, background_tasks: Ba
form_data = FormModel.model_validate(body)
json_data = form_data.model_dump_json()

print("\n\n\n", json_data)
# print("\n\n\n", json_data)

# Ugh, I'd like to find a more efficient way to get the user data. But alas, that
# the sqlalchemy-signing table is not optimized alongside the user model...
Expand Down Expand Up @@ -613,13 +616,26 @@ async def api_form_update(form_name: str, document_id: str, background_tasks: Ba
if config.COLLECT_USAGE_STATISTICS:
metadata[DocumentDatabase.ip_address_field] = request.client.host

# Process the validated form submission as needed
_ = DocumentDatabase.update_document(
form_name=form_name,
document_id=document_id,
json_data=json_data,
metadata=metadata
)
try:
# Process the validated form submission as needed
success = DocumentDatabase.update_document(
form_name=form_name,
document_id=document_id,
json_data=json_data,
metadata=metadata
)

# Unlike other methods, like get_one_document or fuzzy_search_documents, this method raises exceptions when
# it fails to ensure the user knows their operation was not successful.
except DocumentDoesNotExist as e:
raise HTTPException(status_code=404, detail=f"{e}")

except DocumentIsDeleted as e:
raise HTTPException(status_code=410, detail=f"{e}")

except InsufficientPermissions as e:
raise HTTPException(status_code=403, detail=f"{e}")


# Send email
if config.SMTP_ENABLED:
Expand Down
41 changes: 32 additions & 9 deletions libreforms_fastapi/utils/document_database.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,25 @@ def __init__(self, form_name):
message = f"The collection '{form_name}' does not exist."
super().__init__(message)

class DocumentDoesNotExist(Exception):
"""Exception raised when attempting to access a document that does not exist."""
def __init__(self, form_name, document_id):
message = f"The document with ID '{document_id}' collection '{form_name}' does not exist."
super().__init__(message)

class DocumentIsDeleted(Exception):
"""Exception raised when attempting to access a document that has been deleted."""
def __init__(self, form_name, document_id):
message = f"The document with ID '{document_id}' collection '{form_name}' has been deleted and cannot be edited."
super().__init__(message)

class InsufficientPermissions(Exception):
"""Exception raised when attempting to access a document that user lacks permissions for."""
def __init__(self, form_name, document_id, username):
message = f"User '{user}' has insufficinet permissions to perform the requested operation on document" \
f"with ID '{document_id}' collection '{form_name}'."
super().__init__(message)


# Pulled from https://github.com/signebedi/gita-api
def fuzzy_search_normalized(text_string, search_term, segment_length=None):
Expand Down Expand Up @@ -358,49 +377,53 @@ def update_document(self, form_name:str, document_id:str, json_data:str, metadat
if not document:
if self.use_logger:
self.logger.warning(f"No document for {form_name} with document_id {document_id}")
return None
raise DocumentDoesNotExist(form_name, document_id)

# If exclude_deleted is set, then we return None if the document is marked as deleted
if exclude_deleted and document['metadata'][self.is_deleted_field] == True:
if self.use_logger:
self.logger.warning(f"Document for {form_name} with document_id {document_id} is deleted and was not updated")
return None
raise DocumentIsDeleted(form_name, document_id)

# If we are limiting user access based on group-based access controls, and this user is
# not the document creator, then return None
if isinstance(limit_users, str) and document['metadata'][self.created_by_field] != limit_users:
if self.use_logger:
self.logger.warning(f"Insufficient permissions to update document for {form_name} with document_id {document_id}")
return None
raise InsufficientPermissions(form_name, document_id, limit_users)

current_timestamp = datetime.now(self.timezone)

print("\n\n\n\nDocument: ", document)
# print("\n\n\n\nDocument: ", document)

# This is a little hackish but TinyDB write data to file as Python dictionaries, not JSON.
updated_data_dict = json.loads(json_data)

# Here we remove data that has not been changed
dropping_unchanged_data = {}
for field in updated_data_dict.keys():
print(field)

# print(field)

if all([
field in document['data'].keys(),
updated_data_dict[field] != document['data'][field],
updated_data_dict[field] is not None
]):

print(f"\n\n\n{field} in document data but has different value")
# print(f"\n\n\n{field} in document data but has different value")

dropping_unchanged_data[field] = updated_data_dict[field]
elif all([
field not in document['data'].keys(),
updated_data_dict[field] is not None
]):

print(f"\n\n\n{field} not in document data")
# print(f"\n\n\n{field} not in document data")

dropping_unchanged_data[field] = updated_data_dict[field]

print("\n\n\nDropping Unchanged Fields: ", dropping_unchanged_data)
# print("\n\n\nDropping Unchanged Fields: ", dropping_unchanged_data)

# Build the journal
journal = document['metadata'].get(self.journal_field)
Expand All @@ -425,7 +448,7 @@ def update_document(self, form_name:str, document_id:str, json_data:str, metadat
document['metadata'][self.journal_field] = journal


print("\n\n\nUpdated Document: ", document)
# print("\n\n\nUpdated Document: ", document)

# Update only the fields that are provided in json_data and metadata, not replacing the entire
# document. The partial approach will minimize the room for mistakes from overwriting entire documents.
Expand Down

0 comments on commit 85fbafa

Please sign in to comment.