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

fix: Consistent response format #259

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

Conversation

ekas-7
Copy link

@ekas-7 ekas-7 commented Mar 15, 2025

Standardize API Response Format

Description

This PR addresses issue #119 "Streamline and harmonize simple responses across API routes" by implementing a consistent response format across all endpoints in the Folksonomy API.

Previously, the API returned responses in various formats:

  • Simple string responses like "ok"
  • JSON with {"detail": "message"}
  • Nested JSON with {"detail": {"msg": "message"}}

This inconsistency made it difficult for API consumers to handle responses predictably.

Changes Made

  1. Added a SimpleResponse model in models.py with a standardized structure:

    class SimpleResponse(BaseModel):
        detail: Dict[str, str]
  2. Updated the PingResponse model to follow the same structure.

  3. Modified all endpoints that return simple responses to use this consistent format:

    • POST /product endpoint (create)
    • PUT /product endpoint (update)
    • DELETE /product/{product}/{k} endpoint (delete)
    • GET /ping endpoint
  4. Updated error responses to follow the same format.

  5. Fixed empty array responses to return [] instead of null when no data is found.

  6. Added response_model=SimpleResponse parameter to appropriate API routes for better documentation.

Example Changes

Before:

return "ok"  # or return {"message": "ok"}, etc.

After:

return {"detail": {"msg": "Tag created successfully"}}

@ekas-7 ekas-7 requested a review from a team as a code owner March 15, 2025 14:40
@ekas-7 ekas-7 changed the title chore: Consistent response format fix: Consistent response format Mar 15, 2025
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 86.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 94.48%. Comparing base (b9cda65) to head (cef15cf).
Report is 75 commits behind head on main.

Files with missing lines Patch % Lines
folksonomy/api.py 81.81% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #259      +/-   ##
==========================================
- Coverage   95.06%   94.48%   -0.58%     
==========================================
  Files           5        5              
  Lines         324      381      +57     
==========================================
+ Hits          308      360      +52     
- Misses         16       21       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@alexgarel alexgarel left a comment

Choose a reason for hiding this comment

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

@ekas-7 per se it's a good idea, but as already said on other PRs we can't change the API without first verifying it does not break:

  • the mobile app
  • the open food facts website.

So to propose this PR, you must verify if it's causing problem for them, or not.

@ekas-7
Copy link
Author

ekas-7 commented Mar 21, 2025

I will propose this in an issue @alexgarel in both repos

@ekas-7
Copy link
Author

ekas-7 commented Mar 22, 2025

Opened an issue in smooth app
openfoodfacts/smooth-app#6468
for this @alexgarel

@ekas-7
Copy link
Author

ekas-7 commented Mar 22, 2025

Opened in Web as well
openfoodfacts/openfoodfacts-web#696
for this @alexgarel

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.

3 participants