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.

Sign up for GitHub

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

Jump to bottom

feat(TripPlanner): Show more itineraries #2278

Merged
merged 1 commit into from
Dec 19, 2024
Merged

feat(TripPlanner): Show more itineraries #2278

merged 1 commit into from
Dec 19, 2024

Conversation

joshlarson
Copy link
Contributor

Screenshot 2024-12-19 at 2 58 11 PM

Summary of changes

Before

  • We retrieved 5 itineraries from OTP, then grouped them, and returned them however they were grouped.
  • This sometimes led to all of the itineraries being shown as one group, or two groups, and sometimes left out certain itineraries.
    • For instance, Logan Airport to North Station can easily be done with the Back Bay Logan Express shuttle, followed by an OL transfer, but if that wasn't in the next 5 trips, then it wouldn't show up.

After

  • We retrieve 100 itineraries from OTP, group those, and then within each group, surface the next five itineraries (so as not to overwhelm).

Asana Ticket: Increase number of itineraries returned from 5

@joshlarson joshlarson requested a review from a team as a code owner December 19, 2024 20:01
@joshlarson joshlarson enabled auto-merge (squash) December 19, 2024 20:03
@thecristen thecristen added the dev-blue Deploy to dev-blue label Dec 19, 2024
@thecristen thecristen removed the dev-blue Deploy to dev-blue label Dec 19, 2024
@joshlarson joshlarson merged commit c35dbff into main Dec 19, 2024
26 of 27 checks passed
@joshlarson joshlarson deleted the jdl/more-itineraries branch December 19, 2024 21:05
anthonyshull
anthonyshull reviewed Dec 21, 2024
View reviewed changes
test/dotcom/trip_plan/itinerary_groups_test.exs
@@ -34,6 +34,21 @@ defmodule Dotcom.TripPlan.ItineraryGroupsTest do
assert Kernel.length(grouped_itineraries) == 1
end

test "only includes the first five itineraries in a group", %{stops: [a, b, c]} do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like how this new test sticks with the pattern of other tests in the file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anthonyshull anthonyshull anthonyshull left review comments

@thecristen thecristen thecristen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@joshlarson @anthonyshull @thecristen