Skip to content

Add Places Serializer Field#49

Open
minifisk wants to merge 15 commits into
oscarmcm:masterfrom
minifisk:add-serializer
Open

Add Places Serializer Field#49
minifisk wants to merge 15 commits into
oscarmcm:masterfrom
minifisk:add-serializer

Conversation

@minifisk
Copy link
Copy Markdown

@minifisk minifisk commented Dec 30, 2023

This PR is expected to cater those users building a API with Django-rest-framework and want to use the location field in their responses.

This PR introduces the PlacesSerializerField to improve the handling of geolocation data in our Django application. The field manages comma-separated geolocation strings, providing structured output and ensuring data integrity.

Key Changes:

  • Added PlacesSerializerField in serializers.py, supporting the format "country,city,latitude,longitude".
  • Implemented tests for both to_representation and to_internal_value methods.
  • Updated project dependencies to include djangorestframework.

Testing:

  • Comprehensive tests have been added to validate the functionality of PlacesSerializerField, covering various input scenarios.

Dependencies:

  • Added djangorestframework to enhance API capabilities.

@minifisk minifisk marked this pull request as draft December 30, 2023 14:04
@minifisk minifisk marked this pull request as ready for review December 30, 2023 14:25
@minifisk minifisk changed the title Add Custom Places Serializer Field Add Places Serializer Field Dec 30, 2023
@oscarmcm oscarmcm self-requested a review January 2, 2024 14:59
@oscarmcm oscarmcm self-assigned this Jan 2, 2024
@oscarmcm
Copy link
Copy Markdown
Owner

oscarmcm commented Jan 2, 2024

Wow!! this looks cool! I'm gonna add some fixes so we can make the rest-framework dependency optional.

Copy link
Copy Markdown
Owner

@oscarmcm oscarmcm left a comment

Choose a reason for hiding this comment

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

LGTM!

@minifisk
Copy link
Copy Markdown
Author

minifisk commented Jan 2, 2024

Can't merge due to commits needed to be verified. Didn't manage to set this up on my computer, maybe can solve this in some way? @oscarmcm

@oscarmcm
Copy link
Copy Markdown
Owner

oscarmcm commented Jan 2, 2024

Yeah I'll merge it locally and/or squash all the commits into one

@minifisk
Copy link
Copy Markdown
Author

minifisk commented Jan 7, 2024

Commited a fix, causing wrong order of city and country

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants