farmOS / field-kit

A modular, offline-first companion app to farmOS.
https://farmOS.org
GNU General Public License v3.0
62 stars 39 forks source link

Log and asset changed filter is too restrictive #527

Open paul121 opened 11 months ago

paul121 commented 11 months ago

Describe the bug Giving this a go with farmOS 3.x and all is working well, but in the process noticed one thing: no data was being returned with asset and log requests, but I know there are assets and logs in my test instance. After inspecting the API requests in the console I believe this is because the changed filter is simply too restrictive: instead of = I think this should be >, so we request all entities that were changed "after" the timestamp, not "at the" timestamp.

Request URL:

https://3.x.ddev.site/api/log/observation?filter[timestamp-0-filter][condition][path]=timestamp&filter[timestamp-0-filter][condition][operator]=>&filter[timestamp-0-filter][condition][value]=1696112413&filter[timestamp-1-filter][condition][path]=timestamp&filter[timestamp-1-filter][condition][operator]=<&filter[timestamp-1-filter][condition][value]=1700000413&filter[changed-0-filter][condition][path]=changed&filter[changed-0-filter][condition][operator]==&filter[changed-0-filter][condition][value]=1698704339

And console view of query params: changed-filter

Copying this URL into my browser and changing the changed-0-filter operator to > returns data as expected.

I'm not entirely sure, but I think this is the relevant code in FK? https://github.com/farmOS/field-kit/blob/6365afd2c52ed17802f21203ef81f016bf845c39/packages/field-kit/src/idb/cache.js#L170

jgaehring commented 11 months ago

Thanks, Paul! Just glancing at this super fast, but I assume this is not related to farmOS/farmOS.js#86, correct?

I'm not entirely sure, but I think this is the relevant code in FK?

Ah, yea, could probably be remedied by changing it to:

if (settings.lastSync) filter.changed = { $gt: settings.lastSync };

Or $gte, just to be generous.

paul121 commented 11 months ago

Correct, I do not think that this is related to https://github.com/farmOS/farmOS.js/issues/86 - each of the filters here have unique IDs so that seems to be working well.

jgaehring commented 5 months ago

Once I wrap up the other remaining issues for the alpha.9 milestone, I'll see if this can be resolved easily enough, like I suggested it might above, and try to tack this on before tagging alpha.9. So I'm including this in the milestone for now. If it doesn't turn out to be as easy as all that, however, I may bump this back to the beta.1 release.