-
Notifications
You must be signed in to change notification settings - Fork 292
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
feat: Integrate Recent Stops Data into External Surveys #1234
Conversation
- Added functionality to include recent stop information in external survey forms. Signed-off-by: Amr Hossam <[email protected]>
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.
couple questions, small change requests. generally, though, this looks fantastic!
object RecentStopsManager { | ||
|
||
// Maximum stops count to save | ||
private var MAX_STOP_COUNT = 5 |
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.
👍
|
||
withContext(Dispatchers.IO) { | ||
val existingRegion = regionDao.getRegionByID(regionId) | ||
if (existingRegion == null) { |
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.
what happens when properties of a region change? (e.g. the server URL changes or the support email address changes?)
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.
I don't save any of them I only save the region ID, currently, this table is customized for the survey feature only and can be extended in the future.
onebusaway-android/src/main/java/org/onebusaway/android/database/recentStops/dao/RegionDao.kt
Outdated
Show resolved
Hide resolved
@@ -231,6 +232,7 @@ public IntentBuilder(Context context, String stopId) { | |||
public IntentBuilder(Context context, ObaStop stop, HashMap<String, ObaRoute> routes) { | |||
mIntent = new Intent(context, ArrivalsListFragment.class); | |||
mIntent.setData(Uri.withAppendedPath(ObaContract.Stops.CONTENT_URI, stop.getId())); | |||
RecentStopsManager.saveStop(context,stop); |
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.
<3
Signed-off-by: Amr Hossam <[email protected]>
Signed-off-by: Amr Hossam <[email protected]>
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.
great work! 🚢
Added functionality to include recent stop information in external survey forms.
Apply the
AndroidStyle.xml
style template to your code in Android Studio.Run the unit tests with
gradlew connectedObaGoogleDebugAndroidTest
to make sure you didn't break anythingIf you have multiple commits please combine them into one commit by squashing them for the initial submission of the pull request. When addressing comments on a pull request, please push a new commit per comment when possible (reviewers will squash and merge using GitHub merge tool)