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

Relations and services point to each other #144

Merged
merged 2 commits into from
Mar 5, 2021

Conversation

senhalil
Copy link
Contributor

  • Prevent uninitialized has_many objects (e.g. vrp.services, vrp.relations, vehicle.rests, etc.) returning a different empty array object with each call
  • Instead of ids, use actual references inside the relations and vice versa

@senhalil senhalil marked this pull request as draft February 19, 2021 16:33
@senhalil
Copy link
Contributor Author

Converted to draft because some tests are broken on Travis after I added the vehicle part as we discussed this morning.

It looks like there are vehicle ids in the relation[:linked_vehicle_ids] list which does not exist. I am not sure if it is broken test logic or a bug due to the location of "expansion" of vehicle trips..

Gonna check next week.

@senhalil senhalil force-pushed the feat/relation_references branch 2 times, most recently from 455d5c6 to ea7f7c7 Compare February 25, 2021 13:49
@senhalil
Copy link
Contributor Author

Moved vehicle-relation references to #146 since it needs a change in the multi-tour expansion location but the relation-service references are good to go.

@senhalil senhalil changed the title Relations point to the actual objects Relations and services point to each other Feb 25, 2021
@senhalil senhalil marked this pull request as ready for review February 25, 2021 14:00
@senhalil senhalil force-pushed the feat/relation_references branch from ea7f7c7 to ef57b46 Compare March 1, 2021 09:28
@fonsecadeline
Copy link
Contributor

@senhalil are you talking about multitour expansion from multitrips.expand ? I think we talked about moving this expand within vrp.create but I can not find related comments.

@senhalil
Copy link
Contributor Author

senhalil commented Mar 1, 2021

@senhalil are you talking about multitour expansion from multitrips.expand ? I think we talked about moving this expand within vrp.create but I can not find related comments.

Yes, I was referring to MultiTrips::expand. Maybe we talked it at the daily point, I don't remember.

Once that function is moved to the beginning of vrp.create -- before the "vrp.send"s -- we can go ahead with the other PR (#146) at the moment creation of relations poses problem because the vehicles "don't exist" from the point of view of base.rb.

@fonsecadeline
Copy link
Contributor

I see, I will work on this expand move as soon as #142 and #143 are merged (prerequisits for multi tour dev)

@fab-girard fab-girard merged commit e355183 into Mapotempo:dev Mar 5, 2021
@senhalil senhalil deleted the feat/relation_references branch March 10, 2021 16:31
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.

4 participants