UnionInternationalCheminsdeFer / OSDM

Projects related to an open sales & distribution API for public transportation.
https://osdm.io
Apache License 2.0
53 stars 21 forks source link

Making confirmableUntil optional in booking response #369

Closed stinacarlstedt closed 9 months ago

stinacarlstedt commented 10 months ago
image

In 3.0.3 the value is mandatory, which is correct in pre-booked state but not relevant after confirmation. The suggestion is to make the field optional, with docuementation that it is expected before confirmation. It should not be nullable, to avoid implementation issues in handling dates.

jspetrak commented 10 months ago

@stinacarlstedt One clarification - we have to distinguish between not required and nullable in openapi spec. The sooner means that the property is always provided in the object but value can be NULL, the later allows both for null value and completely omit the property which would be breaking on API structure level.

Enabling null value in post-prebooking state is breaking on business logic level but we can treat this as just a clarification of behavior.

jspetrak commented 10 months ago

Ehm, I am just once again looking into the definition and we may not need any change. Because the property already allows the null value. Let's discuss tomorrow.

edit:

linking explanation because the docs is a bit misleading https://stackoverflow.com/a/45577763

CGantert345 commented 10 months ago

Joseph to test generators whether it should be nullable or not. The element will become optional.

jspetrak commented 9 months ago

Josef to provide PR based on the test for 24.11.

jspetrak commented 9 months ago

When I removed required, @NotNull validation was removed by the generator.

Setting nullable to true wrapped the property into JsonNullable which servers as indicator, if the property is present in the JSON or not. But it does not handle if that has or has not the null value.

Having required true and nullable true does not work correctly.

Handling null value is not a problem for any serious developer.

I would suggest to make it required false in order to allow null value for confirmed bookings.

jspetrak commented 9 months ago

Based on presentation of code generated on 2023-11-24, the solution is to remove "required" constraint that allows to have confirmableUntil with null value. Backend validation must be performed to always provide/obtain value for PREBOOKED booking parts.

PR was provided.