Produits Entr'ouvert » Authentic 2

Voici 2 tests qui valideront que les champs dates réagissent comme le champ text.
(J'ai noté là où le comportement diffère actuellement.)

def test_api_users_required_date_attributes(settings, app, admin, simple_user):
    Attribute.objects.create(kind='string', name='prefered_color', label='prefered color', required=True)
    Attribute.objects.create(kind='date', name='date', label='date', required=True)
    Attribute.objects.create(kind='birthdate', name='birthdate', label='birthdate', required=True)

    User = get_user_model()
    payload = {
        'username': simple_user.username,
        'id': simple_user.id,
        'email': 'john.doe@nowhere.null',
        'first_name': 'Johnny',
        'last_name': 'Doe',
    }
    headers = basic_authorization_header(admin)
    resp = app.put_json('/api/users/{}/'.format(simple_user.uuid),
            params=payload, headers=headers, status=400)
    assert resp.json['result'] == 0
    assert resp.json['errors']['prefered_color'] == ['This field is required.']
    assert resp.json['errors']['date'] == ['This field is required.']
    assert resp.json['errors']['birthdate'] == ['This field is required.']

    payload['prefered_color'] = 'blue'
    payload['date'] = '1515-1-15'
    payload['birthdate'] = '1900-2-22'
    resp = app.put_json('/api/users/{}/'.format(simple_user.uuid),
            params=payload, headers=headers, status=200)
    resp = app.get('/api/users/{}/'.format(simple_user.uuid),
            headers=headers, status=200)
    assert resp.json['prefered_color'] == 'blue'
    assert resp.json['date'] == '1515-01-15'
    assert resp.json['birthdate'] == '1900-02-22'

    payload['prefered_color'] = ''
    payload['date'] = ''
    payload['birthdate'] = ''
    resp = app.put_json('/api/users/{}/'.format(simple_user.uuid),
            params=payload, headers=headers, status=400)
    assert resp.json['result'] == 0
    assert resp.json['errors']['prefered_color'] == ['This field may not be blank.']
    assert resp.json['errors']['date'] == ['This field may not be blank.']           # <- ici (Date has wrong format...)
    assert resp.json['errors']['birthdate'] == ['This field may not be blank.']      # <- ici (idem)

def test_api_users_optional_date_attributes(settings, app, admin, simple_user):
    Attribute.objects.create(kind='date', name='date', label='date', required=False)
    Attribute.objects.create(kind='birthdate', name='birthdate', label='birthdate', required=False)

    User = get_user_model()
    payload = {
        'username': simple_user.username,
        'id': simple_user.id,
        'email': 'john.doe@nowhere.null',
        'first_name': 'Johnny',
        'last_name': 'Doe',
    }
    headers = basic_authorization_header(admin)
    resp = app.put_json('/api/users/{}/'.format(simple_user.uuid),
            params=payload, headers=headers, status=200)
    resp = app.get('/api/users/{}/'.format(simple_user.uuid),
            headers=headers, status=200)
    assert 'prefered_color' not in resp.json.keys()
    assert 'date' not in resp.json.keys()                 # <- ici
    assert 'birthddate' not in resp.json.keys()           # <- ici

    payload['prefered_color'] = ''
    payload['date'] = ''
    payload['birthdate'] = ''
    resp = app.put_json('/api/users/{}/'.format(simple_user.uuid),
            params=payload, headers=headers, status=200)             # <- ici (Date has wrong format...)
    resp = app.get('/api/users/{}/'.format(simple_user.uuid),
            headers=headers, status=200)
    assert 'prefered_color' not in resp.json.keys()
    assert 'date' not in resp.json.keys()
    assert 'birthddate' not in resp.json.keys()

Désolé, le second test ci-dessus est faux : j'ai oublié d'ajouter l'attribut témoin "prefered_color".

En fait, le comportement observé correspond à celui défini par Django-Rest-Framework : rien n'est prévu pour les dates nulles.
https://www.django-rest-framework.org/api-guide/fields/#datefield

Le patch proposé ajoute un champ "allow_blank" au type DateField de Django-Rest-Frameword, afin d'harmoniser le comportement des champs d'A2. 'date' et 'birthdate' se comportent à présent (sur ce point) comme le champs 'string'.

Pour accepter la chaîne vide, je suis intervenu avant la fonction virtuelle 'run_validation' (qui appelle la méthode 'to_internal_value' vérifiant le format ISO_8601).

D'autres options sont envisageables : surcharger 'to_internal_value' ou encore dériver de CharField (ici j'ai préféré copier/coller le minimum requis depuis rest_framework/fields.py::CharField).

Oui, bien vu (j'ai du le rajouter à un moment où ça ne passait pas, alors que mon patch n'était pas abouti).

J'avais un doute sur le fait qu'il me manque le pendant de la correction côté CSV.
Notamment, comme pour #35647.

api_views.py:

class BaseUserSerializer(serializers.ModelSerializer):
...
    def __init__(self, *args, **kwargs):
        ...
        for at in Attribute.objects.all():
            if at.name in self.fields:
                self.fields[at.name].required = at.required
                if at.required and isinstance(self.fields[at.name], serializers.CharField):
                    self.fields[at.name].allow_blank = False
++              elif at.required and isinstance(self.fields[at.name], DateRestField):
++                  self.fields[at.name].allow_blank = False

Mais en fait on boucle ici sur les camps 'canoniques' : (à priori) ceux de RegistrationSerializer. Or comme cette classe ne contient n'a pas de champ date, cet ajout serait en fait du code mort.

Hors sujet, mais je ne comprend pas encore pourquoi ce patch n'a pas d'impact sur les tests CSV.
Par exemple, je m'attendais à ce qu'une date avec une chaîne blanche (ex: ' ') soit désormais acceptés, mais ce n'est pas le cas.

Nicolas Roche a écrit :

• classe DateRestField est copiée/collée depuis rest_framework/fields.py::CharField
  Je pense que c'est mieux de garder le code le plus proche possible, notamment pour le message d'erreur qui sinon différera de celui des autres champs.

Ok, j'avais pas vu ça.

• classe Attribute est copiée/collée depuis le code de Benjamin.
  Idem je préfère garder le code en l'état pour ne pas altérer le 'git blame'

À mon avis c'est justifié de factoriser les appels à issubclass, mais c'est du détail, fais comme tu veux.

commit 05340b110b7d0cb9ba2a30a8b88fb6b22d7e56e6
Author: Nicolas ROCHE <nroche@entrouvert.com>
Date:   Tue Sep 24 19:17:20 2019 +0200

    api: extend DRF date field to accept empty string (#36365)