Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce Schemas to handle missing body, empty transaction list, missing fields, additional date formats #48

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

MattFaz
Copy link
Collaborator

@MattFaz MattFaz commented Nov 24, 2024

Added Schema functionality to allow easier validating of requests and responses.
No body, missing transactions, or missing fields will now return appropriate error messages (see screenshots attached).

image
image
image

@MattFaz
Copy link
Collaborator Author

MattFaz commented Nov 24, 2024

Transactions can now handle multiple date formats:

  • MMM DD, YYYY
  • MMM DD YYYY
  • YYYY-MM-DD (ISO)

image

@MattFaz MattFaz changed the title Introduce Schemas to handle no body, missing body, missing fields Introduce Schemas to handle missing body, empty transaction list, missing fields, additional date formats Nov 24, 2024
@@ -20,9 +16,3 @@ async def get_api_key(api_key_header: str = Security(api_key_header)):
status_code=403,
detail="Could not validate credentials",
)


def generate_custom_id():
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed as its only used in 1 function, and we can use the default python uuid library

from services.actual_service import actual_service

router = APIRouter()


@router.post("/transactions", dependencies=[Depends(get_api_key)])
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed as /transaction and /transactions/ will work regardless, no need to add both.
Additionally we do not need the dependency on get_api_key here as it's already added in main.py when we add the route

@MattFaz
Copy link
Collaborator Author

MattFaz commented Nov 25, 2024

@bobokun happy to discuss anything here :)

@MattFaz MattFaz requested a review from bobokun November 25, 2024 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant