entur / lamassu

Mobility hub
European Union Public License 1.2
5 stars 7 forks source link

fix: do not skip vehicles without pricing plan id (fixes #396) #397

Closed hbruch closed 5 months ago

hbruch commented 5 months ago

Summary

This PR changes VehicleFilter to not skip vehicles with their pricing_plan_id not set

Issue

Closes #396

Unit tests

A unit test to demonstrate this issue was added.

Bumping the serialization version id

n.a.

testower commented 5 months ago

I'm happy with this provided this comment is also addressed: https://github.com/entur/lamassu/issues/396#issuecomment-2020570284

hbruch commented 5 months ago

I'm happy with this provided this comment is also addressed: #396 (comment)

Done with 74ff16d (manually tested)

testower commented 5 months ago

I just realized there’s another snag. Since pricingPlan is non-nullable in the graphql schema, and defaultPricingPlan in vehicle_types.json is optional prior to v2.3, we still need to filter out those vehicles that lack pricingPlanId and whose vehicleType lacks a defaultPricingPlanId, to avoid a very ugly error in the graphql api should you be unlucky to query for vehicles with their pricing plans and stumbling upon one of these.

hbruch commented 5 months ago

027e091e76d6bf4dbf20d637c7cf43c1e120b1f2 now checks if, for vehicles without pricing plan, the vehicle type has a default pricing plan. This covers cases in which the default_pricing_plan_id does not exist in pricing plans, which is mapped to a null defaultPricingPlan.

Besides this, I changed the skipped log messages to be more concise. Now they only print the relevant ids.

testower commented 5 months ago

Looks good! Rebase or merge master branch and it should be good to go :)