Fixed resetting finished audiobooks

[MitjaHenner]

Stops progress of 0.0 to get synced when the chapter is not the first.
Issue: #337

When the last chapter is finished runSync gets called with the currentChaperIndex of the last chapter but with a currentTotalTime of 0.0.
This sync resets the audiobooks progress and changed it from finished back to unfinished.

[GrakovNe]

Magic numbers and magic values

currentChapterIndex > 1 is a magic boundary. No one knows why it is > 1 and not >= 0, > 0, etc. without digging into the business rules.

0.0 is a magic value that implicitly means “user hasn’t started yet”, but this meaning is encoded in a raw primitive instead of being expressed via a named constant, a value object, or a domain‑specific type.

This makes the logic tightly coupled to hidden assumptions in your head rather than something the code communicates explicitly to the next reader.

[MitjaHenner]

Magic numbers and magic values

currentChapterIndex > 1 is a magic boundary. No one knows why it is > 1 and not >= 0, > 0, etc. without digging into the business rules.

0.0 is a magic value that implicitly means “user hasn’t started yet”, but this meaning is encoded in a raw primitive instead of being expressed via a named constant, a value object, or a domain‑specific type.

This makes the logic tightly coupled to hidden assumptions in your head rather than something the code communicates explicitly to the next reader.

My bad, I debugged using an audiobook with a super short first chapter, so it looked like the index would start at 1 when debugging. It should be 0.

Is there a reason to ever sync a currentTotalTime of 0.0? (Even if the first chapter is playing)

[GrakovNe]

Is there a reason to ever sync a currentTotalTime of 0.0? (Even if the first chapter is playing)

Yes, we do have a real use case for syncing currentTotalTime = 0.0.

A user may explicitly want to reset their progress in a book. The way this is currently done in the app is by seeking to the very beginning (00:00), even if it’s the first chapter. From the client perspective, that is an intentional state change, not just an implicit “not started yet” situation.

If we ignore 0.0, we lose the ability to propagate that reset across devices, and the previous progress may be restored unintentionally.

That said, I’m not 100% sure how critical this is in practice, but the reset-to-zero scenario is a valid and user-driven action on our side.

[GrakovNe]

I’m still a bit concerned that this looks like “magic logic”.

Why is this the correct rule:

don’t sync currentTotalTime == 0.0 unless it’s the first chapter

gated by currentIndex > FIRST_CHAPTER

What is the empirical basis for this behavior? Was it derived from a specific reproducible bug, or from observed incorrect sync scenarios?

At the moment, it feels like a narrowly scoped workaround that solves a concrete issue, but also hard-codes an implicit assumption into the sync layer: that 0.0 is invalid progress except for chapter 0. That assumption is not self-evident and may be fragile long term (especially considering chapter/file mapping, explicit progress reset, cross-device sync races, etc.).

I would be more comfortable if we could clearly articulate the invariant we’re trying to enforce here, rather than encoding it indirectly through positional checks.

[MitjaHenner]

Yea it would be cleaner to not trigger this sync at all.

Right now the problem is:
Last chapter of an audiobook is getting played and the audiobook goes to finished once there are only a few seconds left in the audiobook.

Once the last chapter ended a few sync calls get triggered where the chapterIndex is still the last chapter but currentTotalTime is 0.0 this sync resets the progress on the server.

Changing it to isLastChapter && 0.0 -> return would do the same but still cause problems with single chapter audiobooks I guess.