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

fix(shuttles): Fix indexes changing to 0-based and displayed stop names changing when stops are reordered #1090

Merged
merged 5 commits into from
Jan 17, 2025

Conversation

rudiejd
Copy link
Contributor

@rudiejd rudiejd commented Jan 14, 2025

Summary of changes

Asana Ticket: [extra] 🏹🐛 Map view - stops index changing (from first stop as 0 or as 1

Problem:
When re-ordering shuttle stops for a new shuttle, the indexing of stops would change from 1-based to 0-based after the first reordering.

Solution:
Start using 1-based indexing for stops in the stop_input component.

Problem:
When re-ordering shuttle stops for a new shuttle, stops with auto-completed names would switch from the auto completed value (e.g. 1 - Washington St opp Ruggles St) to just the value of the stop ID e.g. just 1

Solution:
On every re-render of stops, ensure that the value of the stop_input component is the human readable value. I fetch from the stop from the changeset if it's already there, otherwise I fetch the stop from the DB.

Reviewer Checklist

  • Meets ticket's acceptance criteria
  • Any new or changed functions have typespecs
  • Tests were added for any new functionality (don't just rely on Codecov)
  • This branch was deployed to the staging environment and is currently running with no unexpected increase in warnings, and no errors or crashes.

@rudiejd rudiejd requested review from a team and meagharty and removed request for a team January 14, 2025 22:30
if stop == nil do
{text, text}
else
option_for_stop(stop)
Copy link
Contributor

@meagharty meagharty Jan 17, 2025

Choose a reason for hiding this comment

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

not blocking: I'd love to have :display_stop / stop_or_gtfs_stop_for_stop_id consolidated in some way but I think this function hints at the (existing) issue where we were deal with the changes and the committed values across new and edit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree - I feel like display_stop is probably unnecessary since the stop is probably loaded in the changeset by the time the user has entered it. In the interest of keeping this PR small and making iterative improvements, I'm going to hold off on that for now.

Copy link
Contributor

@meagharty meagharty left a comment

Choose a reason for hiding this comment

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

thanks for fixing up both of these bugs!

@rudiejd rudiejd force-pushed the fix/map-view-stops-index-changing branch from 4858e64 to 3db3e64 Compare January 17, 2025 18:18
@rudiejd rudiejd merged commit 54f6efb into master Jan 17, 2025
10 checks passed
@rudiejd rudiejd deleted the fix/map-view-stops-index-changing branch January 17, 2025 18:27
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.

2 participants