-
Notifications
You must be signed in to change notification settings - Fork 52
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
🚲 Bike station migration 🚲 : Moving to mobility API (v2) #470
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Part of the migration to use mobility v2 api to fetch bike rental stations * The new API returns data of the type Station, with different properties
* .. to be used with objects of the TranslatedString type from entur/sdk * enables easier language-change if future translations are added to the API
Fixed render loop in useEffect
* Fixed missing bike-tile * Refactored useEffect into smaller responsibilities
Changed implementation to use custom useBikeRentalStations hook in EditTab
* Migrating to fetch from mobility API v2 in BikeSearch * Type migration BikeRentalStation -> Station
* added argument removeHiddenStations which may be set to false to enable usability for EditTab where hidden stations is needed in toggle-list * fixed filtering logic in hook (removed adding of duplicate stations)
* Fixed conditionless adding of bike rental station added through search field. Now checks if ID is already stored in settings.newStations
draperunner
reviewed
Sep 21, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bra jobba!
Eg tenker squashing er greit, for WIP-commits har ofte ikkje så mykje verdi med mindre dei kan stå for seg sjølv.
* Removed includesStation() from utils.tsx, implemented logic directly in useBikeRentalStation
* Changed cleanups in useEffects (in EditTab/*) to use AbortController instead of primitive boolean variable
draperunner
approved these changes
Sep 21, 2021
mjansrud
reviewed
Sep 21, 2021
mjansrud
reviewed
Sep 21, 2021
Credz for fin PR-beskrivelse 🙌 |
* Default to empty string '' instead of station.id * Sorting station names defaults to putting stations with missing name last
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🚲
This PR migrates the part of the app considering bike stations to utilize the mobility v2 API using the entur SDK.
The reason why many files are changed is primarily due to the different format of the returned data using the type
Station
. The primary changes are in the custom hookuseBikeRentalStations()
, the majority of other file changes are adaptations to the new data format.The commit-log includes multiple Work-In-Progress commits (due to unknown migration scope) which may be squashed for prettier history. The drawback being a big bulk of changes with less traceability.
Changes:
Rewrite useBikeRentalStation() to use mobility v2
As an effect we move from using BikeRentalStation type to Station type (return type of relevant mobility v2 functions)
Rewrite EditTab to use mobility v2 where needed
Different naming system (
getTranslation()
utility function added)station.name
→station.name.translation[0].value
'nb'
language ID (norsk bokmål)Other:
StopPlaceSearch
Dropdown
componentDiscovered "bug" (fix incoming): User selected stations (Fixed in additional commitnewStations
in settings) stores duplicates if same stations is searched and selected more than once.