Skip to content

Fixed odometer reading to update properly - #413

Open
ShaniDan wants to merge 3 commits into
mikaelacaron:mainfrom
ShaniDan:dev
Open

Fixed odometer reading to update properly#413
ShaniDan wants to merge 3 commits into
mikaelacaron:mainfrom
ShaniDan:dev

Conversation

@ShaniDan

Copy link
Copy Markdown

What it Does

How I Tested

  • Ran the application
  • Verified that the userID is present by putting breakpoint and debugging it with po in the debugger

Notes

Screenshot

@ShaniDan
ShaniDan requested a review from mikaelacaron as a code owner July 18, 2026 00:49

@mikaelacaron mikaelacaron left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Great work!! I have several comments that need resolved please. If you have any questions, you can comment directly on that thread, otherwise click the resolve button once you've addressed my comment. Once you fix everything, please click the re-review button

I'm getting an error in the console. Also a few comments of some stuff to fix please!

Attempting to initialize or set a @documentid property with a non-nil value: "docIDHere". The document ID is managed by Firestore and any initialized or set value will be ignored. The ID is automatically set when reading from Firestore.

Image

Comment on lines +195 to +197
.onAppear {
Task { await viewModel.getVehicles() }
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This should use .task instead


let firebaseService: FirebaseServiceProtocol

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

remove some of these line breaks / tabs that were added on accident

vin: VIN,
licensePlateNumber: licensePlateNumber)
addTapped(vehicle)
print("VEHICLE HAS BEEN ADDED")

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

remove this please! cause it's just debugging

Comment on lines +61 to +63
if let uid = userUID {
var readingToUpdate = reading
readingToUpdate.userID = uid

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

something similar is happening in FirebaseService.updateReading() should that be removed?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

That's the question I have.
Why a separate FirebaseService class is used for only OdometerReading because it can be done in OdometerViewModel itself just like it's done for the DshboardViewModel

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It's to be able to unit test OdometerViewModel

Eventually both OdometerViewModel and DashboardViewModel will use a FirebaseService

@ShaniDan
ShaniDan requested a review from mikaelacaron July 22, 2026 01:03
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.

BUG - Odometer Reading Doesn't Update Properly

2 participants