feat: RoutePatterns table, part 1: Adding to db

[hannahpurcell]

Posting this for early review (tests and migration safety to come. I'm eager to work on serialization next.)

This doesn't yet cover retrieval from the db (serialization), because that body of work was easily separated from db insertion.

Needs:

• Tests
• Fixing migration safety

[hannahpurcell]

Something I just realized in Part 2 of this task: I'm throwing away the "garages" field on the route. Maybe we should be storing that in the db, just because? I dunno, maybe not, because it's very possible that field will never get used by the Detours feature

[firestack]

Something I just realized in Part 2 of this task: I'm throwing away the "garages" field on the route. Maybe we should be storing that in the db, just because? I dunno, maybe not, because it's very possible that field will never get used by the Detours feature

I think we should update the createDetourMachine context to match our new shape in the database too, or at least tell it that garages doesn't exist any more, so that we don't write any code that depends on it.

I like the idea of storing everything, but lets make the assumption for now that garages are a "now" thing and not something that we need to store?

[firestack]

based on our directions.txt extension I'm hesitant to "prematurely" restrict this, but I'd be open to this if we can confirm that bus always has only Inbound and Outbound.

[hannahpurcell]

Ahhhh rats. I didn't realize direction could be so varied in gtfs. I do believe we as an agency embrace only the Inbound / Outbound, so I could confirm that with TD.

On the other hand, I had a related dilemma. I just realized that we need to be able to repopulate this field in the state machine: "directionNames" => %{"0" => "Outbound", "1" => "Inbound"} so that we can create this component

I'll need to adjust the db schema accordingly. Maybe store the map and the direction id instead of the direction name

[firestack]

We shouldn't need to store that info on a Detour as it's only relevant when we're first picking the route pattern right?

That's a good point though that we'll want to somehow gate the "route pattern picker" part based on some criteria, like if the route they're viewing is no longer current. A detour shouldn't be saved to the database in this state though. So we should be able to discard this info and make the state machine refetch this info.

[hannahpurcell]

Ah that's right! Let me think about this

[hannahpurcell]

Ok had to doublecheck:

it's only relevant when we're first picking the route pattern right?

Unfortunately, no, because of this line in DiversionPage:
const routeDirection = routePattern && route.directionNames[routePattern.directionId]

which used at every stage of the detour-creation workflow. So that will need to be refactored a bit on the frontend, which will necessitate some more tweaks on the backend

[firestack]

I wouldn't spend too much time on it if it's tricky, but I think you could reference Schedule.Gtfs.RoutePattern.id() and Schedule.Gtfs.Route.id() in the schema via https://hexdocs.pm/typed_ecto_schema/TypedEctoSchema.html#module-overriding-the-typespec-for-a-field

[firestack]

And if you need to create new type aliases that'd be fine.
As for where they'd live, I think starting out with them in the Ecto Schema file is fine?

[hannahpurcell]

Gottttttt it, ok. I'm having a little trouble, but expect to figure it out. But in the meantime, follow-up question: does enforcing this schema on input also enforce it on retrieval? (That might impact whether I try to get through the trickiness or just use strings.) Also do we have any examples of using custom types in the field of a typed_schema?

[firestack]

I'm not sure we have an example, but I'm picturing something like this, I've not tested it though and can't confirm it works as is.

field(:gtfs_route_pattern_id, :string) :: Schedule.Gtfs.RoutePattern.id()

[hannahpurcell]

Ahhhh I see, thank you. I'll test it now. The typed_schema definitely had some magic going on that prevented putting the RoutePattern within the field call.

Edit: Yes, that works!

[firestack]

I didn't know this existed! I think we should set writable: :insert on these fields so we don't accidentally update them

Suggested change

	field(:gtfs_route_pattern_id, :string)
	field(:gtfs_route_pattern_name, :string)
	field(:gtfs_route_pattern_headsign, :string)
	field(:gtfs_route_pattern_direction_name, DirectionName)
	# The given route pattern will always have the given route
	# Does it need to be stored here as well?
	field(:gtfs_route_id, :string)
	field(:gtfs_route_name, :string)
	field(:gtfs_route_pattern_time_description, :string)
	field(:gtfs_route_pattern_id, :string, writable: :insert)
	field(:gtfs_route_pattern_name, :string, writable: :insert)
	field(:gtfs_route_pattern_headsign, :string, writable: :insert)
	field(:gtfs_route_pattern_direction_name, DirectionName, writable: :insert)
	# The given route pattern will always have the given route
	# Does it need to be stored here as well?
	field(:gtfs_route_id, :string, writable: :insert)
	field(:gtfs_route_name, :string, writable: :insert)
	field(:gtfs_route_pattern_time_description, :string)