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

[BO - Signalement] Correction de l'édition de la composition du logement avec une superficie vide #2131

Merged
merged 2 commits into from
Jan 22, 2024

Conversation

emilschn
Copy link
Collaborator

Ticket

#2104

Description

Quand la superficie n'est pas renseignée, le formulaire plantait parce qu'il attendait un null|float

Tests

  • Editer un signalement en modifiant la superficie avec différents types de données

@@ -422,7 +422,7 @@ public function updateFromInformationsLogementRequest(Signalement $signalement,
public function updateFromCompositionLogementRequest(Signalement $signalement, CompositionLogementRequest $compositionLogementRequest)
{
$signalement->setNatureLogement($compositionLogementRequest->getType());
$signalement->setSuperficie($compositionLogementRequest->getSuperficie());
$signalement->setSuperficie(!empty($compositionLogementRequest->getSuperficie()) ? $compositionLogementRequest->getSuperficie() : null);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Du coup on peut ajouter un test sur cette méthode avec la superficie à null ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

tu parles d'un test fonctionnel ? ou ajouter un test dans setSuperficie ? ou dans getSuperficie ? ou autre ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oui mais sans la couche http juste avec la base de données.

  • Récupérer un nouveau signalement des fixtures
  • Tester updateFromCompositionLogementRequest avec des valeurs qui peuvent être à null comme superficie dans CompositionLogementRequest.

@numew
Copy link
Collaborator

numew commented Jan 19, 2024

J'aurais préféré une correction via l'utilisation des contrainte de validation, qu'en pensez-vous ?
Idéalement à mettre en place aussi sur le nouveau formulaire

@emilschn
Copy link
Collaborator Author

J'aurais préféré une correction via l'utilisation des contrainte de validation, qu'en pensez-vous ? Idéalement à mettre en place aussi sur le nouveau formulaire

D'après moi, on peut laisser le champ vide, si on ne sait pas. Donc je ne sais pas comment on gèrerait au niveau des contraintes de validation ?

@numew
Copy link
Collaborator

numew commented Jan 19, 2024

J'aurais préféré une correction via l'utilisation des contrainte de validation, qu'en pensez-vous ? Idéalement à mettre en place aussi sur le nouveau formulaire

D'après moi, on peut laisser le champ vide, si on ne sait pas. Donc je ne sais pas comment on gèrerait au niveau des contraintes de validation ?

Les contrainte s'applique uniquement si la valeur est renseigné (a moins d rajouter la contrainte spécifique NotBlank), je pense que la plus adéquate serait celle la https://symfony.com/doc/current/reference/constraints/Positive.html

@emilschn
Copy link
Collaborator Author

emilschn commented Jan 19, 2024

Ok donc du coup, c'est un test complémentaire à ce que j'ai fait ? Pas en remplacement :)
Si oui, je suis d'accord !

Oups j'avais mal saisie le soucis désolé, les contraintes régleront pas le problème.
Vu que les DTO nous retourne un string est-ce que le mieux serait pas d'accepter ca dans le setSuperficie() de Signalement et de faire la conversion à l'intérieur ?
Ca me gène un peu de le faire plus haut car le problème va être emmené a se reproduire via un autre appel de la fonction

Oui, ok, ça me convient !

@emilschn
Copy link
Collaborator Author

@sfinx13 @numew j'ai pris en compte vos retours

Copy link

sonarcloud bot commented Jan 19, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Collaborator

@hmeneuvrier hmeneuvrier left a comment

Choose a reason for hiding this comment

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

Relecture et tests OK

@sfinx13 sfinx13 merged commit 30bf4ea into develop Jan 22, 2024
2 of 3 checks passed
@emilschn emilschn deleted the fix/2104-empty-superficie branch February 6, 2024 14:18
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.

None yet

4 participants