fix(AWAN-149): show local data first and never keep deleted data - #45
Merged
mSaayeh merged 9 commits intoAug 10, 2026
Merged
Conversation
Zones edited in profile never reached Home until the day changed. Two independent causes, both instances of a wider gap: AWAN-205 declared Room the single read model but only carried it through for sessions and the profile. - Add ZoneDao.observeEffectiveZonesForDate: one query whose subqueries span zones, template_overrides and template_days_of_week, so Room re-emits on any of them. Home's schedule now combines it with the sessions flow instead of resolving zones with suspend one-shots inside the map, which Room only invalidated for the sessions table - Add ZonesLocalDataSource as the only writer of the four zone tables; its replaceAll deletes templates and overrides first, so zones and day assignments removed on another device cannot survive a refresh - Refresh the zone model into Room after every one of the 12 zone mutations, which is also what makes the three delete mutations correct - Fail syncZonesAndTemplates when either GET fails instead of reporting success unconditionally and writing half a replace - Refresh zones and the visible day's schedule on Home open and on date change, forced past the TTL; days outside the background week were never fetched at all before - Write through deleteSession, deleteTask, updateTaskDetails and updateSessionLock into Room, and add the connectivity guard all four lacked - Clear every cached table on logout and when a different account signs in; Room is keyed by server ids alone, so the previous account's rows read back as the new one's and the backend answers 404 for ids it does not own - Restore the zone categoryId wiring that the AWAN-205 merge dropped from OnboardingRepositoryImpl and ZonesRepositoryImpl, and un-weaken the guard test that was edited to pass against the broken version - Refill categories from the network when Room is empty; onboarding needs them before SyncWorker has run, and skipping the setup used to outrun the fetch - Decode v1/goals/inbox into bare tasks instead of task/session pairs - Document the contract in CLAUDE.md with a three-grep review checklist, and record the traps in the deferred tables in docs/feature/offline-first/
mSaayeh
requested review from
Abdallah-Elsobky,
SherifAshraf2020,
ZeiadT and
esraaehab333
as code owners
August 10, 2026 05:25
Moving a session updated the UI, then snapped back, and Room never changed. - extractTimeFromIso tried OffsetDateTime then LocalTime, and the API sends neither: "2026-08-09T14:30:00" has no offset and is not a bare time. Every caller silently kept its fallback, which in cacheSession is the session's existing time — so a moved session was rewritten with the time it already had. The sync path never showed it because SessionDto.toEntity uses substring instead, so the read and write paths disagreed on the same field - Add the LocalDateTime attempt, and truncate to seconds so sub-second precision cannot leak into an HH:mm:ss column
- Add HomeLocalDataSource as the single owner of the Home feature's Room reads and writes; HomeRepositoryImpl no longer holds a DAO, so what the timeline caches is decided in one file - Move the session cache rule there and cover it directly: a moved session stores its new times, one dragged past midnight changes its date, a response without a status keeps the stored one, and an uncached row reports that it wrote nothing instead of returning silently - Update the row's date on a move; a session dragged to another day used to keep the old day's date and disappear from both - Collapse five DAO fakes in HomeRepositoryImplTest into one
- Replace the four-shape repository contract table with the working rules - Keep the write-through, connectivity-gate, and per-row TTL guidance - Drop the review-greps and the plan-doc pointer now the work has landed
- Keep HomeLocalDataSource as the single owner of Home's Room access; develop's cacheSession, applySessionChange and deleteSession write-throughs land there instead of through DAOs the repository no longer holds - Take develop's getSessionDetail cache fallback, routed through the local source, and add HomeLocalDataSource.getSession for it - Take develop's mapStatus/mapTaskStatus, which the SessionDetailInfo change to LocalDateTime and SessionStatus requires - Collapse updateSessionLock into applySessionChange, which already carries the connectivity guard and the write-through this branch added - Keep both sides' HomeRepositoryImplTest cases; createRepository takes the connectivity monitor develop's offline tests need - Implement refreshZones in develop's FakeZonesRepository
…ventBus - Inject UserDao into GamificationEventBus and persist points, streak, and maxStreak updates to Room UserEntity on every reward emission - Centralize progress caching in GamificationEventBus so session completion and wheel spin rewards update Room consistently - Remove redundant cacheProgress calls from GamificationRepositoryImpl - Update unit tests in GamificationEventBusTest and HomeRepositoryImplTest to verify Room UserEntity persistence
abdelrahman-rashed-ali
approved these changes
Aug 10, 2026
- Fix status preservation in ZonesMapper: keep cached status when API response omits the field instead of defaulting to SCHEDULED - Fix toEntity() to parse ISO-8601 timestamps with UTC offsets, falling back to epoch on malformed input - Remove unused single-purpose use-cases (CreateTemplateOverride, DeleteSession, DeleteTemplateOverride, GetSessionsByDate, GetTemplateOverrides, UpdateSession) - Fix ZonesLocalDataSource to use replaceAll strategy on upsert instead of insert-or-ignore - Fix HomeRepositoryImpl and SessionRepositoryImpl to propagate errors correctly from remote data sources - Fix AuthRepositoryImpl token refresh to not swallow network errors - Add unit tests covering status-preservation, offset timestamp parsing, and malformed-row fallback in ZonesMapperTest - Add unit tests for IsoTimeUtils offset/malformed handling - Add regression tests in AuthRepositoryImplTest for refresh-token error propagation - Add assertion in OfflineSyncCoordinatorTest for replaceAll upsert behaviour
- Add generation to the key() call in rememberDecoratedEntries so that Navigation 3 correctly recomposes entries when the back stack is reset to the same top-level route (e.g. re-selecting the current tab)
ZeiadT
approved these changes
Aug 10, 2026
This file contains hidden or 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
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.
What & Why
A zone edited in the profile screen never appeared on Home until the user changed the day. That has two independent causes, and both are instances of a wider gap: AWAN-205 declared Room the single read model, but the implementation only carried it through for sessions and the profile.
getZonesForDate,getEffectiveZonesand Home all read Room. Profile looked correct only because it reads the network straight into in-memory state.getDaySchedulemapped over the sessions Flow and resolved zones with suspend one-shots inside the map, so Room invalidated it for thesessionstable alone. Changing the day — which builds a new Flow — was the only thing that re-read zones.The deletion half was systemic: of five sync functions only
syncScheduleRangeremoved anything, so a template, override, zone, goal or category deleted on another device never disappeared here.This PR fixes the reported bug, applies the rule to the zones and Home verticals, and writes the rule into
CLAUDE.mdso it is reviewable rather than remembered.Key Changes
ZoneDao.observeEffectiveZonesForDate— one query whose subqueries spanzones,template_overridesandtemplate_days_of_week, so Room's invalidation tracker covers all three. Home nowcombines it with the sessions Flow; the duplicated resolution logic inZonesRepositoryImplcollapses into it.ZonesLocalDataSource.replaceAll— the only writer of the four zone tables. Deletes templates and overrides first (zones and day rows CASCADE), so nothing removed elsewhere survives a refresh.refreshZones()— one shared refresh rather than 12 write-throughs, and the only version where the threedelete*mutations are correct with no extra code.syncZonesAndTemplatesreturns false when either GET fails — it previously returned success unconditionally and could write half a replace.deleteSession,deleteTask,updateTaskDetailsandupdateSessionLocknow land in Room, and got the connectivity guard all four lacked.404 CATEGORY_NOT_FOUNDon zone writes.categoryIdwiring the AWAN-205 merge dropped fromOnboardingRepositoryImplandZonesRepositoryImplis back, and the guard test that had been edited to pass against the broken version now exercises it again.SyncWorkerhas run, and skipping the setup used to outrun the fetch.v1/goals/inboxdecodes into bare tasks instead of{task, sessions}pairs.extractTimeFromIsotriedOffsetDateTimethenLocalTime; the API sends neither ("2026-08-09T14:30:00"), so every caller silently kept its fallback — which incacheSessionis the session's existing time. Moving a session rewrote it with the time it already had: the card snapped back and Room never changed. The sync path hid this becauseSessionDto.toEntityusessubstring, so the read and write paths disagreed about the same field.HomeLocalDataSourceowns every Room read and write for Home;HomeRepositoryImplholds no DAOs. The repository test went from five DAO fakes to one.CLAUDE.mdgains the offline-first contract (four method shapes, replace-never-merge, provable scope, one local data source per table group) with a three-grep review checklist. Traps for the deferred tables are recorded indocs/feature/offline-first/2026-08-09-replace-on-refresh.md.Related
AWAN-149. Extends
docs/feature/offline-first/2026-08-06-offline-first-ssot-plan.md(AWAN-205) and restores work from AWAN-125.Testing
New coverage:
ZoneDaoTest— override-over-template resolution, and the load-bearing one: a single live subscription re-emits after a zone edit, a day reassignment, and a new override. This is the assumption the whole design rests on and it has not been executed yet (needs a device).TemplateDaoTest—deleteAllTemplatescascades days and template zones while leaving override zones intact.ZonesRepositoryImplTest— parameterised over all 12 mutations: each writes to Room exactly once, a failed one writes nothing, offline returnsAppError.Networkwithout touching the network.HomeRepositoryImplTest—getDaySchedulere-emits when only the zones change (the regression test for the reported bug).OfflineSyncCoordinatorTest—syncZonesAndTemplateswrites nothing and returns false when either call fails.IsoTimeUtilsTest— offset-free, zoned, time-only, fractional and unparseable inputs.HomeLocalDataSourceTest— a moved session stores its new times (verified failing against the old parser), a cross-midnight move changes the date, a status-less response keeps the stored status, an uncached row reports it wrote nothing, and deleting a task clears its dependencies.On a device: edit a zone in profile → Home updates without a day change; delete a template on one device → it is gone on the other rather than merged back; airplane mode → everything still renders from cache and writes report the network error; install over an existing build → data survives.
Notes for Reviewers
fallbackToDestructiveMigration(dropAllTables = true)is armed, so this deliberately touches only queries and repository code.ScheduleSynchronizer(so a repository depends on one coordinator method rather than its fifteen data sources) andLocalDataCleaner(AwanDatabaseis an abstract Room class a JVM test cannot construct, and the project has no mocking library).tasks.categoryIdis aNO_ACTIONFK sodeleteAllCategories()throws a constraint error, andlistGoalsis page 0 withincludeInbox = false— replacing from it deletes the user's Inbox goal. Both are written up in the feature doc.TaskRepositoryImpl,AiTaskRepositoryImpl,TemplateRepositoryImpl,GoalRepositoryImpl,CategoryRepositoryImpl,ProfileRepositoryImpl,OnboardingRepositoryImplandInventoryRepositoryImplstill inject DAOs directly, andOfflineSyncCoordinatorstill holds seven. Until it calls the per-feature local sources instead, the "zeroupsertin the coordinator" grep inCLAUDE.mdis aspirational rather than true.