WordPress-FluxC-Android
WordPress-FluxC-Android copied to clipboard
Querying Activity Log events returns only a subset of Site's events
Current behavior
This is a pretty tricky issue. Sometimes Activity Log screen in WPAndroid displays only a subset of available events for the site. The app fetches first 100 events from the server, however, sometimes the app displays only eg. 4 events even though the server response contained 100 events.
What we found
- The server returns the correct set of data so this is a bug in the WPAndroid app (FluxC)
- All fetched data are correctly passed to
insertOrUpdateActivities
- ActivityLogTable contains events with identical
REMOTE_SITE_ID
but differentLOCAL_SITE_ID
. Only one of thoseLOCAL_SITE_ID
is present in SiteModelTable. - Even items which are referencing the invalid/out-dated
LOCAL_SITE_ID
are stored inactivitiesToUpdate
. -
updateQueries
filters out entries with invalidLOCAL_SITE_ID
which leads to this issue.
Example
Imagine your local database looks like the below table and "fetchActivities" returns activities with activity_id 1-8.
This line returns events with activity_id 1-7. Event with activity_id = 8 is inserted into database on https://github.com/wordpress-mobile/WordPress-FluxC-Android/blob/7ba5682b47cfd0a0bcb43aed045d5b0d4c9a984f/fluxc/src/main/java/org/wordpress/android/fluxc/persistence/ActivityLogSqlUtils.kt#L36 as it doesn't exist in the local db. Items with activity_id 1-7 should be inserted on this line but only item with activity_id = 7 gets actually inserted/updated since the rest doesn't match this condition. The UI will show only activity_id 7-8 as the remaining items are referencing invalid local_site_id.
SiteModelTable | ||
---|---|---|
id (aka local site id) = 10 | siteId(aka remote site id) = 101010 | |
ActivityLogTable | ||
local_site_id = 7 | remote_site_id = 101010 | activity_id = 1 |
local_site_id = 7 | remote_site_id = 101010 | activity_id = 2 |
local_site_id = 7 | remote_site_id = 101010 | activity_id = 3 |
local_site_id = 7 | remote_site_id = 101010 | activity_id = 4 |
local_site_id = 7 | remote_site_id = 101010 | activity_id = 5 |
local_site_id = 7 | remote_site_id = 101010 | activity_id = 6 |
local_site_id = 10 | remote_site_id = 101010 | activity_id = 7 |
We are not sure how to get into the state where the ActivityLogTable contains items with an invalid local_site_id (or not invalid, but referencing an id which doesn't exist anymore in the SiteModel table).
Fix proposals:
Since we are not able to get the app into this invalid state again, the following proposals might be not be the best possible solutions. However, we have verified that at least fix proposal 1
fixed the issue.
- Add
.equals(ActivityLogTable.LOCAL_SITE_ID, it.localSiteId)
below this line to make sure we are querying only events which are referencing the corresponding site (existing local_site_id). - Make sure that we purge ActivityLogTable when we are removing an entry from SiteModelTable -> for our example, we'd delete events in ActivityLogTable when we were deleting site with local_site_id = 7 from the SiteModelTable. I'm not sure where we should put this code since I'm not sure from when/who deleted the site with local_site_id = 7 from the SiteModelTable.
Props to @ParaskP7 for reporting this issue and jumping on a call and helping to investigate what is going on. Props to @ashiagr If I recall correctly, you encountered this issue a few weeks/months ago and your findings were similar to ours (it helped us quickly find and verify the behavior)
cc @planarvoid Do you by any chance remember why this line contains the "local_site_id" check but this line doesn't? My guess is that it was a mistake and the same check should have been added even to the second expressions. But I thought I'd better ping you just in case I'm missing something.
Do you by any chance remember why this line contains the "local_site_id" check but this line doesn't? My guess is that it was a mistake and the same check should have been added even to the second expressions. But I thought I'd better ping you just in case I'm missing something.
I have no idea @malinajirka but it looks like a mistake :-)