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

Fixed #895 - Allow saving for routes into the DB #1152

Closed

Conversation

amrhossamdev
Copy link
Member

@amrhossamdev amrhossamdev commented Mar 9, 2024

Fixes #895

Details

After a couple of hours investigating the issue and the DB and trying to debug the saving into the DB, I found that class ObaContract is responsible for the CRUD operations and after reviewing the class code I found that an inner class called Routes that was having a function called InsertOrUpdate and sure the class responsible for managing CRUD operations of recent routes and I found that this method isn't used when showing a route on the map / showing trip statues / searching for a route and of course, saving will not work in this case.

Fix

I wrote two functions in the DBUtil class that will work to save the route when showing it showing trip statues / or clicking on the route in search results. and called this function when all of these operations are used and now it's working perfectly.

Video showing trying different test cases

Screencast.from.03-09-2024.01.47.30.PM.webm
  • 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 anything

  • If 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)

unused import.
…sue-895-recent-routes

# Conflicts:
#	onebusaway-android/src/main/java/org/onebusaway/android/util/DBUtil.java
Copy link
Member

@aaronbrethorst aaronbrethorst left a comment

Choose a reason for hiding this comment

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

Excluding some minor refactoring in the database layer, I think this looks great and is ready to go.

Copy link
Member

@aaronbrethorst aaronbrethorst left a comment

Choose a reason for hiding this comment

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

great work!

@aaronbrethorst
Copy link
Member

I'm going to merge this in PR #1175 because GitHub is complaining about merge conflicts.

image

@amrhossamdev
Copy link
Member Author

Thank you !

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.

Recent routes isn't populated as often as it should be
2 participants