base-adresse: provide ordering options to cities endpoint (#74374) #103
|
@ -6,6 +6,7 @@ from io import StringIO
|
|||
from urllib import parse as urlparse
|
||||
|
||||
from django.contrib.postgres.fields import JSONField
|
||||
from django.core.exceptions import FieldError
|
||||
from django.db import connection, models
|
||||
from django.db.models import Q
|
||||
from django.utils import timezone
|
||||
|
@ -346,9 +347,19 @@ class BaseAdresse(AddressResource):
|
|||
},
|
||||
'region_code': {'description': _('Region code'), 'example_value': '11'},
|
||||
'department_code': {'description': _('Department code'), 'example_value': '75'},
|
||||
'ordering': {
|
||||
'description': _(
|
||||
|
||||
'Comma-separated ordering field list (the fields are "name", "unaccent_name", '
|
||||
'"code", "zipcode", "population", "department" and "region", and can each be '
|
||||
'prefixed with "-" for reverse ordering)'
|
||||
),
|
||||
'example_value': '-population,zipcode,unaccent_name,name',
|
||||
},
|
||||
},
|
||||
)
|
||||
def cities(self, request, id=None, q=None, code=None, region_code=None, department_code=None):
|
||||
def cities(
|
||||
self, request, id=None, q=None, code=None, region_code=None, department_code=None, ordering=None
|
||||
):
|
||||
cities = self.citymodel_set.all()
|
||||
|
||||
if id is not None:
|
||||
|
@ -374,6 +385,13 @@ class BaseAdresse(AddressResource):
|
|||
cities = cities.filter(department__code=department_code)
|
||||
|
||||
cities = cities.select_related('department', 'region')
|
||||
Ghost
commented
Ce "if" n'est pas utile, car Ce "if" n'est pas utile, car `cities.order_by(*ordering.split(','))` fonctionne aussi avec une chaîne sans virgule.
pmarillonnet
commented
Bien vu, en effet, j’avais loupé ça. C’est corrigé, c’est plus simple ainsi. Bien vu, en effet, j’avais loupé ça. C’est corrigé, c’est plus simple ainsi.
|
||||
|
||||
Ghost
commented
Ici il faut juste modifier la query_set cities et laisser la suite (le return des cities) s'appliquer tranquillement. A voir d'ailleurs d'il faut faire le select_related('department', 'region') avant ou après. Ici il faut juste modifier la query_set cities et laisser la suite (le return des cities) s'appliquer tranquillement. A voir d'ailleurs d'il faut faire le select_related('department', 'region') avant ou après.
pmarillonnet
commented
Le problème est que le FieldError n’est pas levé lors du order_by (comportement “lazy”), seulement lorsque le sql est réellement exécuté, c’est-à-dire lors de
Le problème est que le FieldError n’est pas levé lors du order_by (comportement “lazy”), seulement lorsque le sql est réellement exécuté, c’est-à-dire lors de
> return {'data': [city.to_json() for city in cities]}
|
||||
if ordering:
|
||||
try:
|
||||
ordered = cities.order_by(*ordering.split(','))
|
||||
return {'data': [city.to_json() for city in ordered]}
|
||||
Ghost
commented
en mode pénible, je dirais de sortir le return du try/except en mode pénible, je dirais de sortir le return du try/except
|
||||
except FieldError:
|
||||
Ghost
commented
Il faut faire une APIError qui retourne une 400 (un self.logger.error ça fait surtout une trace sentry, non merci ;) ) Il faut faire une APIError qui retourne une 400 (un self.logger.error ça fait surtout une trace sentry, non merci ;) )
pmarillonnet
commented
Ok, j’avais mélangé ces deux choses. C’est corrigé dans la branche. Ok, j’avais mélangé ces deux choses. C’est corrigé dans la branche.
|
||||
raise APIError(f'cities: erroneous ordering query {ordering}')
|
||||
return {'data': [city.to_json() for city in cities]}
|
||||
|
||||
@endpoint(
|
||||
|
|
|
@ -157,6 +157,19 @@ def city(db, region, department):
|
|||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def city2(db, region, department):
|
||||
return CityModel.objects.create(
|
||||
name='Aix-les-Bains',
|
||||
code='73008',
|
||||
zipcode='73010',
|
||||
population=30000,
|
||||
region=region,
|
||||
department=department,
|
||||
resource=BaseAdresse.objects.first(),
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def miquelon(db):
|
||||
return CityModel.objects.create(
|
||||
|
@ -565,7 +578,7 @@ def test_base_adresse_command_update_multiple(mocked_get, db, base_adresse_multi
|
|||
assert StreetModel.objects.count() == 5
|
||||
|
||||
|
||||
def test_base_adresse_cities(app, base_adresse, city, miquelon, department, region):
|
||||
def test_base_adresse_cities(app, base_adresse, city, city2, miquelon, department, region):
|
||||
resp = app.get('/base-adresse/%s/cities?q=chambe' % base_adresse.slug)
|
||||
assert len(resp.json['data']) == 1
|
||||
result = resp.json['data'][0]
|
||||
|
@ -581,8 +594,11 @@ def test_base_adresse_cities(app, base_adresse, city, miquelon, department, regi
|
|||
assert result['department_name'] == city.department.name
|
||||
|
||||
resp = app.get('/base-adresse/%s/cities?q=73' % base_adresse.slug)
|
||||
assert len(resp.json['data']) == 1
|
||||
assert len(resp.json['data']) == 2
|
||||
assert resp.json['data'][0] == result
|
||||
assert resp.json['data'][1]['name'] == city2.name
|
||||
assert resp.json['data'][1]['zipcode'] == city2.zipcode
|
||||
assert resp.json['data'][1]['id'] == '%s.%s' % (city2.code, city2.zipcode)
|
||||
|
||||
resp = app.get('/base-adresse/%s/cities?code=73065' % base_adresse.slug)
|
||||
assert len(resp.json['data']) == 1
|
||||
|
@ -593,6 +609,37 @@ def test_base_adresse_cities(app, base_adresse, city, miquelon, department, regi
|
|||
assert resp.json['data'][0] == result
|
||||
assert resp.json['data'][1]['name'] == 'Miquelon-Langlade'
|
||||
|
||||
# default ordering, descending number of inhabitants
|
||||
resp = app.get('/base-adresse/%s/cities?department_code=73' % base_adresse.slug)
|
||||
assert len(resp.json['data']) == 2
|
||||
assert resp.json['data'][0]['name'] == city.name
|
||||
assert resp.json['data'][1]['name'] == city2.name
|
||||
|
||||
# sorted by population (ascending) then name
|
||||
resp = app.get('/base-adresse/%s/cities?department_code=73&ordering=population,name' % base_adresse.slug)
|
||||
assert resp.json['data'][0]['name'] == city2.name
|
||||
assert resp.json['data'][1]['name'] == city.name
|
||||
|
||||
# sorted by name
|
||||
resp = app.get('/base-adresse/%s/cities?department_code=73&ordering=name' % base_adresse.slug)
|
||||
assert resp.json['data'][0]['name'] == city2.name
|
||||
assert resp.json['data'][1]['name'] == city.name
|
||||
|
||||
# sorted by name (reverse) then by code
|
||||
resp = app.get('/base-adresse/%s/cities?department_code=73&ordering=-name,code' % base_adresse.slug)
|
||||
assert resp.json['data'][0]['name'] == city.name
|
||||
assert resp.json['data'][1]['name'] == city2.name
|
||||
|
||||
# sorted by code
|
||||
resp = app.get('/base-adresse/%s/cities?department_code=73&ordering=code' % base_adresse.slug)
|
||||
assert resp.json['data'][0]['name'] == city2.name
|
||||
assert resp.json['data'][1]['name'] == city.name
|
||||
|
||||
# nonexistent ordering field, fallback on default ordering
|
||||
resp = app.get('/base-adresse/%s/cities?department_code=73&ordering=foobar' % base_adresse.slug)
|
||||
assert resp.json['err'] == 1
|
||||
assert resp.json['err_desc'] == 'cities: erroneous ordering query foobar'
|
||||
|
||||
|
||||
def test_base_adresse_cities_missing_region_and_department(app, base_adresse, miquelon):
|
||||
resp = app.get('/base-adresse/%s/cities?q=miqu' % base_adresse.slug)
|
||||
|
|
Ici il faudrait préciser la liste des clés de tri (name, unaccent_name, code, zipcode, population, departement, region) et je mettrais "-population,zipcode,unaccent_name,name" en exemple, sous-entendant que c'est la valeur par défaut, et "documentant" au passage l'usage du "-".
Ok, j’ai ajouté la liste des clés, et j’ai modifié l’exemple comme tu le suggères ici.