Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log and asset changed filter is too restrictive #527

Open
paul121 opened this issue Oct 30, 2023 · 3 comments
Open

Log and asset changed filter is too restrictive #527

paul121 opened this issue Oct 30, 2023 · 3 comments

Comments

@paul121
Copy link
Member

paul121 commented Oct 30, 2023

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?

if (settings.lastSync) filter.changed = settings.lastSync;

@paul121 paul121 added the bug label Oct 30, 2023
@paul121 paul121 changed the title Logs changed filter is too restrictive Log and asset changed filter is too restrictive Oct 31, 2023
@jgaehring
Copy link
Member

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
Copy link
Member Author

paul121 commented Oct 31, 2023

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

@jgaehring
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants