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

Group Location Issue #44

Open
symbioquine opened this issue Jun 9, 2023 · 6 comments
Open

Group Location Issue #44

symbioquine opened this issue Jun 9, 2023 · 6 comments
Labels
bug triage Something isn't working

Comments

@symbioquine
Copy link
Owner

image
I did confirm an issue with group memberships and asset link. It appears that asset link is not taking into account the assets location if it's in a group.and the group has a location.

These are screenshots of the 2 animal assets and the group asset they belong to in farmOS

image
image
image

Here is the same assets in asset link.

image
image
image

@symbioquine symbioquine added the bug triage Something isn't working label Jun 9, 2023
@mstenta
Copy link

mstenta commented Oct 23, 2024

Is asset link computing location itself?

The Group module overrides the asset location logic to take into account group membership.

Notably, assets have a computed location and geometry in the farmOS API. Perhaps Asset Link should load that instead of computing location itself. Then it will always have the same logic.

Although I imagine the reason for computing internally is so that it can work offline? So if you move an animal while you're offline it will show the correct location in Asset Link... is that the intention?

@mstenta
Copy link

mstenta commented Oct 24, 2024

So if you move an animal while you're offline it will show the correct location in Asset Link... is that the intention?

One idea: maybe if Asset Link detects that you are offline, it puts a little warning symbol next to computed fields like Location to indicate that they may not be updated until network connection is re-established.

(This assumes that it moves to using the computed location field of assets from the API rather than computing it itself.)

@mstenta
Copy link

mstenta commented Oct 24, 2024

The Group module overrides the asset location logic to take into account group membership.

Of course, Asset Link could also add logic to mimic the Group module's... but that might be unreasonable to maintain. And it's always possible that other modules could change it in other ways. 😅

@symbioquine
Copy link
Owner Author

Although I imagine the reason for computing internally is so that it can work offline? So if you move an animal while you're offline it will show the correct location in Asset Link... is that the intention?

Yes that's the goal.

Though Asset Link tries not to re-compute the group/location/geometry information unless it has reason to "believe" it needs to:

// Only replace the computed location/geometry fields if at least one of:
// - The asset is newly created
// - We have relevant pending log updates
if (!isNewlyCreatedAsset && !hasRelevantPendingRemoteLogUpdates) {
return;
}

The problems are two-fold;

  1. I can't remember whether I've implemented that transitive group location/movement logic
  2. Even if I have, this could easily be a cache invalidation problem - which are more-or-less inevitable (especially as the depth of relationships that have to be queried grows

@symbioquine
Copy link
Owner Author

One idea: maybe if Asset Link detects that you are offline, it puts a little warning symbol next to computed fields like Location to indicate that they may not be updated until network connection is re-established.

This is actually one area I'm really looking forward to improving Asset Link in...

I would like to provide a more powerful API for interacting with the cache status/info of individual parts of a given farmOS api response. Further I'd also like to make it easier to implement UIs that show the cached data immediately, but (when online) also request the latest data from the server and then surface any new data gracefully (without jumping around as it loads).

@symbioquine
Copy link
Owner Author

  1. I can't remember whether I've implemented that transitive group location/movement logic

From a quick skim of the code, I think the answer is that I haven't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug triage Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants