Is there a need to add more Validations to DTO's?

[BaVilius]

At the moment parsing of Spotted (Date), Id and Severity are done during mapping which is tough to test. I think we should move these parsing operations to DTOValidator classes, so that no parsing would be done during mapping.

[BenasB]

Well parsing in our case is taking in a string and converting into some object – that's also really what we call mapping (converting from one format to other). It's more of a question if we should check the strings if they are in a certain format that the parsing process in the mapper will be able to understand.

What happens now when you try to map PollutedLocationDTO to PollutedLocation when DTO has Id field not empty, but also not a valid guid? Does the mapper throw an exception?

[BaVilius]

It's more of a question if we should check the strings if they are in a certain

I think I agree with you here.

Does the mapper throw an exception?

Yes, I'm pretty sure they are thrown in the mapper. If there is a need, I can create an item, and complete it myself, since I have similiar changes locally.

[BenasB]

Yes, I'm pretty sure they are thrown in the mapper.

It would be wise to add a test here in that case. For example: give a non empty string that is not a guid string and assert that the mapper throws when mapping.

[6ar8nas]

Haven't done much work with the validator, so correct me if I'm wrong. The only option I can think of is performing TryParse when validating DTOs and if it fails, then throw? Are you talking about the same or are there other ways to do it?

EDIT: I can see there are cases of people using it similarly.

[BaVilius]

I personally have the same idea as yours in mind

[BenasB]

Yeah that makes sense, just not throw but return errors with the validator

[BenasB]

Feel free to create an issue yourself and add the rule(s) to DTO validator @BaVilius