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

[8651] kiezradar: add radius based model, serializer, api, view, template #5932

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

m4ra
Copy link
Contributor

@m4ra m4ra commented Jan 13, 2025

Describe your changes
Add a kiezradar model with an optional one2one relation to search profile.
Still missing:

  • serializer validation of kiezradar within Berlin
  • documentation

Tasks

  • PR name contains story or task reference
  • Steps to recreate and test the changes
  • Documentation (docs and inline)
  • Tests (including n+1 and django_assert_num_queries where applicable)
  • Changelog

@m4ra m4ra requested a review from vellip January 13, 2025 19:25
@m4ra m4ra changed the title kiezradar: add radius based model, serializer, api, view, template [8651] kiezradar: add radius based model, serializer, api, view, template Jan 13, 2025
@m4ra m4ra requested a review from goapunk January 13, 2025 19:26
Copy link
Contributor

@goapunk goapunk left a comment

Choose a reason for hiding this comment

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

Thanks, looks good. I left some comments regarding some things. And maybe we want a test for the radius limits (e.g. users can't create a radius < 500 or > 3000 ?. The PR needs a rebase as there are conflicts

meinberlin/apps/kiezradar/models.py Show resolved Hide resolved
meinberlin/apps/kiezradar/models.py Outdated Show resolved Hide resolved
meinberlin/apps/kiezradar/serializers.py Outdated Show resolved Hide resolved
meinberlin/apps/kiezradar/serializers.py Outdated Show resolved Hide resolved
meinberlin/apps/kiezradar/models.py Outdated Show resolved Hide resolved
from .serializers import SearchProfileSerializer


class KiezRadarViewSet(viewsets.ModelViewSet):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@goapunk should we use here the a4 mixin for the permissions? maybe the same for the search profile view

@m4ra m4ra force-pushed the mk-2025-01-kiezradar-api branch from 517af1e to 53b478f Compare January 15, 2025 18:17
@m4ra m4ra force-pushed the mk-2025-01-kiezradar-api branch from 53b478f to 6865b7e Compare January 15, 2025 18:50
@m4ra
Copy link
Contributor Author

m4ra commented Jan 15, 2025

@goapunk I don't have time to add more tests. If you still request changes, could you please take over this PR to finish it as I will be away for 2 weeks and it is needed for @sevfurneaux to continue his work?

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