farmOS / field-kit

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

Maintain a list of farm areas internally #105

Closed mstenta closed 5 years ago

mstenta commented 5 years ago

Broke this out as a separate feature request (and prerequisite of): https://github.com/farmOS/farmOS-client/issues/22

We will need to fetch a list of existing areas from farmOS and store them locally, so that they can be presented as available options to choose from. This will require a separate AJAX request - probably when the app is first initialized (after login credentials are saved for the first time). These areas will need to be stored locally so that they are available as options when you are offline. There will also need to be some way of auto-updating them from time to time - because they may change in farmOS (outside of the app).

EDIT 2018-01-03 by @jgaehring Breaking this down into it's constituent tasks, which I may track in separate issues:

alexadamsmith commented 5 years ago

Currently I am attempting to GET from the same /log URL that we are POSTing to. No success. I get both a 404 error AND a CORS error! Requests from Restlet also return a 404. I need to dig through the plugin that @mstenta wrote in PHP to access the farmOS API, to make sure that I'm pointing at the right resources and using the right headers.

mstenta commented 5 years ago

@alexadamsmith are you familiar with curl?

I have documented how to make requests to the API using curl command on farmOS.org: https://farmos.org/development/api/

It describes in detail what is required for authentication (both getting the cookie, and the token).

mstenta commented 5 years ago

The CORS issue will depend on where you are making the requests to/from.

to test.farmos.net? from locahost:8080?

Or something different?

mstenta commented 5 years ago

Also, when retrieving logs, the endpoint is /log.json (as opposed to /log when POSTing). That's why you are getting 404.

alexadamsmith commented 5 years ago

I'm now successfully getting json data from restlet, but still having the CORS issue with requests from. the app. I am making requests to my local Docker installation of farmOS, running on localhost. I'm trying to get logs from http://localhost/logs.json It works from my browser & restlet browser extension, but not from the app. In my farmOS installation I have the CORS module installed and configured with the following: *|http://localhost:8080||Content-Type,X-Requested-With,Accept,X-CSRF-Token|true The app is running in dev mode on localhost:8080

mstenta commented 5 years ago

Can you post the console error you're getting in the app?

mstenta commented 5 years ago

FWIW, I have a simpler config in my CORS module, although it might not make any difference:

*|http://localhost:8080|||true

alexadamsmith commented 5 years ago

Access to fetch at 'https://test.farmos.net/log.json' from origin 'http://localhost:8080' has been blocked by CORS policy: Response to preflight request doesn't pass access control check: It does not have HTTP ok status.

jgaehring commented 5 years ago

Have you tried it with .json removed, @alexadamsmith?

alexadamsmith commented 5 years ago

Hey @jgaehring - if I remove .json from the endpoint, I get a 404

jgaehring commented 5 years ago

These proxy settings in our webpack config might be a factor too: https://github.com/farmOS/farmOS-native/blob/master/config/index.js#L13

It's very likely adding the endpoint '/log.json' in addition to '/log' might help. You'll need to restart the devServer for it to take effect.

alexadamsmith commented 5 years ago

@mstenta I tried out your simpler CORS config, but unfortunately that blocks my login attempts :(

alexadamsmith commented 5 years ago

I tried starting adding the '/log.json' endpoint to config/index.js; sadly there is still the same CORS issue

alexadamsmith commented 5 years ago

We are back in business - thanks @mstenta and @jgaehring ! We added '/log.json' to the proxy settings, then set the url in Fetch to nothing but '/log.json' .

jgaehring commented 5 years ago

@alexadamsmith , you may want to check this out when you get the chance: https://github.com/farmOS/farmOS-native/blob/master/src/login/loginModule.js#L8

That's how the url is set now within the app. And that's what gets stored. So when you query localStorage for the base url, that's what you'll get back: '' if its in the dev environment, or something like 'http://test.farmos.net' if you're testing it from within Cordova. And if you concatenate your endpoint onto that, it should work whether its the development environment or the production environment; the former will just evaluate to a relative path, the latter to a valid http address.

Also, the docs for the devServer, if you're interested: https://webpack.js.org/configuration/dev-server/#devserver-proxy

alexadamsmith commented 5 years ago

I just made a commit to the http branch of farmos-native with a function in httpModule to get records from the server. Right now the getRecords function is executed when the records are synced to the server. Records obtained are output to the console On line 20 of HttpModule you can specify the kind of record you want to get: 'log' - gets all logs 'taxonomy_term' - gets all taxonomy terms, including areas 'taxonomy_vocabulary' - gets the vid of the area vocabulary. This will be used to filter taxonomy terms down to areas only

I also tried to get the 'asset' resource, but asset.json returns a 404. I tried it out both on my local installation and on test.farmos.net

mstenta commented 5 years ago

I also tried to get the 'asset' resource, but asset.json returns a 404. I tried it out both on my local installation and on test.farmos.net

The endpoint for assets is 'farm_asset.json`. Un-intuitive, I know... might change that in the future.

alexadamsmith commented 5 years ago

Thanks Mike! FYI the API documentation says /asset.json

On Mon, Dec 17, 2018 at 11:48 AM Michael Stenta notifications@github.com wrote:

I also tried to get the 'asset' resource, but asset.json returns a 404. I tried it out both on my local installation and on test.farmos.net

The endpoint for assets is 'farm_asset.json`. Un-intuitive, I know... might change that in the future.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/farmOS/farmOS-client/issues/23#issuecomment-447914650, or mute the thread https://github.com/notifications/unsubscribe-auth/ARAC8NzZMCfSLlasExX82tf-8V2AYrXTks5u58rxgaJpZM4XEY2b .

mstenta commented 5 years ago

Oh oops! Thanks for the heads up - I had that as an outstanding change in my local repo - I must have forgotten to stash it when I ran the site build command, so it got included. I fixed it, so it says /farm_asset.json again.

jgaehring commented 5 years ago

@alexadamsmith, great job! :clap: :clap:

I'm just checking out your commits. I really like how you've generalized it here, and you've commented everything really well.

Perhaps when we get a chance we could do a little code review and submit a PR to get it merged back into master? (Probably when I'm starting sprint 2, so maybe after the holidays, depending what our schedules look like.)

alexadamsmith commented 5 years ago

Thank you Jamie! Just let me know when you're ready, and I will be glad to touch base on the commits.

alexadamsmith commented 5 years ago

Jamie and I went over the getRecords code and talked about how the function could be deployed to get assets and areas from farmOS. I linted and rebased the code and made a new commit.

jgaehring commented 5 years ago

@alexadamsmith , this is great progress! I just pulled your changes into master (https://github.com/farmOS/farmOS-native/pull/72). I think this is another option for rebasing onto master next time, if you open up a PR:

screenshot from 2018-12-27 18-54-37

I didn't actually use that option. Instead, I opened the PR in GitHub, then from my terminal:

git checkout master
git merge http --ff-only
git push origin master

And then GitHub automatically marked it as "Merged". But both are options.

alexadamsmith commented 5 years ago

Sounds good - I can do it as a pull request next time. Thanks!

On Thu, Dec 27, 2018, 7:00 PM Jamie Gaehring <notifications@github.com wrote:

@alexadamsmith https://github.com/alexadamsmith , this is great progress! I just pulled your changes into master (farmOS/farmOS-native#72 https://github.com/farmOS/farmOS-native/pull/72). I think this is another option for rebasing onto master next time, if you open up a PR:

[image: screenshot from 2018-12-27 18-54-37] https://user-images.githubusercontent.com/5556545/50497812-43345f80-0a09-11e9-8c5b-04e74430ca4e.png

I didn't actually use that option. Instead, I opened the PR in GitHub, then from my terminal:

git checkout master git merge http --ff-only git push origin master

And then GitHub automatically marked it as "Merged".

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/farmOS/farmOS-client/issues/23#issuecomment-450257402, or mute the thread https://github.com/notifications/unsubscribe-auth/ARAC8JvuC3TibKKNeBDUHMrpKv1CbAFTks5u9V81gaJpZM4XEY2b .

jgaehring commented 5 years ago

@alexadamsmith I spun up a little test component we can use:

image

jgaehring commented 5 years ago

Ahhh, so this is what we want: https://test.farmos.net/taxonomy_term.json?vocabulary=1

It's all becoming clearer now...

mstenta commented 5 years ago

Yup! But in order to know what "vocabulary" is, you first need to figure out which vocabulary ID corresponds to the vocabulary machine name of "farm_areas". Hence why two requests were necessary... (one to get the list of vocabularies, to find the farm_areas ID, and one to get the list of terms filtered by that ID).

Although... that may change! I just discovered a patch for the restws module which adds the ability to do this in a single request! I tested it and it appears to work. I'm going to integrate it into test.farmOS.net so we can try it out in code... https://www.drupal.org/project/restws/issues/2857490

jgaehring commented 5 years ago

Cool, I've got the chained GET requests working pretty easily, so it shouldn't be too hard to change over if and when the time comes.

mstenta commented 5 years ago

Patched restws in farmOS core: https://github.com/farmOS/farmOS/commit/7df21a07f7fb2ecc226e6cef2563381866268635

mstenta commented 5 years ago

This works now: https://test.farmos.net/taxonomy_term.json?bundle=farm_areas

mstenta commented 5 years ago

Updated docs: https://farmos.org/development/api/#requesting-records

mstenta commented 5 years ago

And updated the farmOS PHP class to use the new method as well: https://github.com/mstenta/farm_sync/commit/070df14f928b537af4786341f4d47c0713150b96

This one was satisfying because it just deletes a bunch of code, for the most part (73 deletions, 4 additions). :-D

jgaehring commented 5 years ago

This one was satisfying because it just deletes a bunch of code, for the most part (73 deletions, 4 additions). :-D

Love those!

jgaehring commented 5 years ago

I'm trying to think right now what properties we need to save from areas, and I wonder if we can get away with just the id and the name. Is there anything else we need in order to associate a log with an area?

When I look at the JSON response from a log with an area associated with it, it looks like this:

"field_farm_area": [
  {
    "uri": "http://localhost/taxonomy_term/9",
    "id": "9",
    "resource": "taxonomy_term"
  }
],
"field_farm_asset": [],
"field_farm_geofield": [
  {
    "geom": "POLYGON ((-75.53737610578538 42.5433662431067, -75.53760141134262 42.54415670203204, -75.53659290075302 42.54437407648197, -75.53620129823686 42.54355200185378, -75.53737610578538 42.5433662431067))",
    "geo_type": "polygon",
    "lat": "42.543854604012",
    "lon": "-75.536932969915",
    "left": "-75.537601411343",
    "top": "42.544374076482",
    "right": "-75.536201298237",
    "bottom": "42.543366243107",
    "srid": null,
    "latlon": "42.543854604012,-75.536932969915",
    "schemaorg_shape": "42.543366243107,-75.537601411343 42.543366243107,-75.536201298237 42.544374076482,-75.536201298237 42.544374076482,-75.537601411343 42.543366243107,-75.537601411343"
  }
]

I'm assuming all we need to send with a log when we post it to the server is the area's taxonomy ID, since everything else, like lat & lon, can be generated from that. The name will only be necessary so we have a human-readable name that can be displayed to the user.

@mstenta, weren't we talking about getting the parent ID so we could nest the areas in the search results? That might be another datum we'd need, if anything.

mstenta commented 5 years ago

I'm trying to think right now what properties we need to save from areas, and I wonder if we can get away with just the id and the name. Is there anything else we need in order to associate a log with an area?

Yea I think that's all we need at a minimum. The tid (term id) is necessary for referencing the area from the log (via the log's farm_field_area field), and the name is necessary for us to show them in a selection form.

@mstenta, weren't we talking about getting the parent ID so we could nest the areas in the search results? That might be another datum we'd need, if anything.

Yes, the parent ID would also be required if we want to show the hierarchical nature of areas in a dropdown of areas, for example. But I think we decided on an autocomplete field, partly to avoid that requirement (since it wouldn't be hierarchical in the autocomplete dropdown) (and also partly because autocomplete can scale to a lot of areas better than a dropdown/select field can). So I don't think parent ID is a high priority right now.

mstenta commented 5 years ago

Also, worth noting (in reference to the JSON code you posted above), the field_farm_geofield field is available on most log types as a way of storing geometry data directly on the log itself (as opposed to referencing the area, which has its own field_farm_geofield geometry - but which may change from time to time).

farmOS has a bit of logic in it that will automatically copy an area's geometry into the log's geometry whenever a log is saved (assuming the log geometry is not explicitly set already).

So... when we reference an area from a log within the app, we should see the field_farm_geofield on the log get populated from the area's geometry when it gets synced to the server.

jgaehring commented 5 years ago

But I think we decided on an autocomplete field, partly to avoid that requirement

Oh right. Sorry if we're repeating earlier conversations, been struggling to find what convo's we already had on the subject. Hopefully having it here now will be a good place for future reference.

So... when we reference an area from a log within the app, we should see the field_farm_geofield on the log get populated from the area's geometry when it gets synced to the server.

Oh that's cool, good to know!

mstenta commented 5 years ago

Yea no worries - I know we've talked some in issues, chat, Hangouts, etc. I can try to be more diligent about getting details like that recorded in GitHub so we have an official record.

alexadamsmith commented 5 years ago

I have an area-related idea that you probably dont need to worry about now, but might consider for the future...

If you were to save those polygon boundary points, it might not be too hard to associate observations with areas based on the device's physical location. If the user is located inside an area and creates an observation, that area could be selected by default in the 'new observation' view. The user could select a different area, or no area, if they prefer. Cordova has a plugon to access device geolocation. I havent used it, but it MAY be straightforward! I can imagine an asynch process that's triggered when the user enters the new record view that goes something like

I know this would take a good chunk of work, and I'm not suggesting we attempt to do it immediately! But if we were to save boundary points for our areas, it would allow us to try this out in the future. I used to work with spatial data back in my crop modeling days, and for me this would be a fun challenge.

That said, if you have already moved ahead with areas as [name, id] I definitely wouldn't stress about it. We could always bring the spatial stuff in later.

Happy 2019!

Alex

On Wed, Jan 2, 2019, 4:24 PM Michael Stenta <notifications@github.com wrote:

Yea no worries - I know we've talked some in issues, chat, Hangouts, etc. I can try to be more diligent about getting details like that recorded in GitHub so we have an official record.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/farmOS/farmOS-client/issues/23#issuecomment-450989693, or mute the thread https://github.com/notifications/unsubscribe-auth/ARAC8AvvsL2YjO2z8x8655QtyGRIKSGxks5u_SOagaJpZM4XEY2b .

jgaehring commented 5 years ago

If you were to save those polygon boundary points, it might not be too hard to associate observations with areas based on the device's physical location.

Yea, this is great! @mstenta and I have talked about something similar, but I think you've laid out the procedure for it really well. Perhaps we can float the idea for Sprint 4?

alexadamsmith commented 5 years ago

A clarification - I just noticed that I refer to a 'new observation' and 'new record' view in my previous email. Both of those are referring to the same thing. I am not at my computer and don't remember what we called the view, but it is what the user sees when they tap 'create a new observation/ record'.

On Thu, Jan 3, 2019, 9:41 AM Alex Smith <alexadamsmith@gmail.com wrote:

I have an area-related idea that you probably dont need to worry about now, but might consider for the future...

If you were to save those polygon boundary points, it might not be too hard to associate observations with areas based on the device's physical location. If the user is located inside an area and creates an observation, that area could be selected by default in the 'new observation' view. The user could select a different area, or no area, if they prefer. Cordova has a plugon to access device geolocation. I havent used it, but it MAY be straightforward! I can imagine an asynch process that's triggered when the user enters the new record view that goes something like

  • Obtain device geolocation
  • Obtain areas from the local DB
  • For each area, check if the device location falls within it's bounds
  • If the device location falls within an area, present that area as a default for the new observation

I know this would take a good chunk of work, and I'm not suggesting we attempt to do it immediately! But if we were to save boundary points for our areas, it would allow us to try this out in the future. I used to work with spatial data back in my crop modeling days, and for me this would be a fun challenge.

That said, if you have already moved ahead with areas as [name, id] I definitely wouldn't stress about it. We could always bring the spatial stuff in later.

Happy 2019!

Alex

On Wed, Jan 2, 2019, 4:24 PM Michael Stenta <notifications@github.com wrote:

Yea no worries - I know we've talked some in issues, chat, Hangouts, etc. I can try to be more diligent about getting details like that recorded in GitHub so we have an official record.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/farmOS/farmOS-client/issues/23#issuecomment-450989693, or mute the thread https://github.com/notifications/unsubscribe-auth/ARAC8AvvsL2YjO2z8x8655QtyGRIKSGxks5u_SOagaJpZM4XEY2b .

mstenta commented 5 years ago

@alexadamsmith Yes Yes YES! Jamie and I discussed this in a brainstorm a while back - it would be an AWESOME feature!

For each area, check if the device location falls within it's bounds

This seems to be the meat of the feature right here. This can be done fairly easily in farmOS, because we have access to the GeoPHP and GEOS libraries, which provide a method of comparing geometries to see if they intersect. So you could say: here is a point (your current geolocation from the device), compare it to all the areas to see which one(s) it intersects with.

So one approach could be to simply add a new API feature to farmOS that let's you simply POST a lat+lon coordinate, and it will return a list of area IDs that intersect it. That could be a nice general feature that is useful in many scenarios... (eg: imagine saying "hey farmOS, show me all input logs that have happened at this point").

The downside of that approach, of course, is that it requires an API request - which means it would require network connection.

In order to do the same thing offline, the client app would need to store all area geometries internally, and have similar geometry intersection detection methods to be able to search through them. Maybe that's not difficult, and maybe there are libraries available for JS that would help (Openlayers may actually provide some methods like that).

If it's difficult, we could consider implementing it as a progressive-enhancement as a first step... works while online, but not while offline, in other words.

jgaehring commented 5 years ago

A clarification - I just noticed that I refer to a 'new observation' and 'new record' view in my previous email.

Haha, no problem. They've actually been renamed to:

Logs.vue
|-- AllLogs.vue
|-- EditLog.vue

Since adding the functionality to toggle the type of log.

alexadamsmith commented 5 years ago

Glad to hear folks are excited about this!!

Mike, you're absolutely right that checking for intersection isn't trivial. I do feel like the feature would be strongest if it could work offline, though a progressive enhancement would be a good second best

I would be happy to look around for JS libraries that could do it for us. Building the functuonality from scratch would be theoretically possible, but probably not easy (I did that kind of thing before, but never in JS)

I'll let you know if I find anything!

Alex

On Thu, Jan 3, 2019, 9:52 AM Michael Stenta <notifications@github.com wrote:

@alexadamsmith https://github.com/alexadamsmith Yes Yes YES! Jamie and I discussed this in a brainstorm a while back - it would be an AWESOME feature!

For each area, check if the device location falls within it's bounds

This seems to be the meat of the feature right here. This can be done fairly easily in farmOS, because we have access to the GeoPHP and GEOS libraries, which provide a method of comparing geometries to see if they intersect. So you could say: here is a point (your current geolocation from the device), compare it to all the areas to see which one(s) it intersects with.

So one approach could be to simply add a new API feature to farmOS that let's you simply POST a lat+lon coordinate, and it will return a list of area IDs that intersect it. That could be a nice general feature that is useful in many scenarios... (eg: imagine saying "hey farmOS, show me all input logs that have happened at this point).

The downside of that approach, of course, is that it requires an API request - which means it would require network connection.

In order to do the same thing offline, the client app would need to store all area geometries internally, and have similar geometry intersection detection methods to be able to search through them. Maybe that's not difficult, and maybe there are libraries available for JS that would help (Openlayers may actually provide some methods like that). If it's difficult, we could consider implementing it as a progressive-enhancement as a first step... works while online, but not while offline, in other words.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/farmOS/farmOS-client/issues/23#issuecomment-451165652, or mute the thread https://github.com/notifications/unsubscribe-auth/ARAC8EbcqTSqWdc_XzuXQD6QbhwKqwEaks5u_hkUgaJpZM4XEY2b .

mstenta commented 5 years ago

Cool! Yea building from scratch is possible, especially if you start simply with a point. It's not too tricky to see if a point is inside a polygon just by comparing the vertices. It gets a little trickier if you consider multi-polygons (where interior shapes are considered NOT part of the polygon), and it gets a whole lot trickier if you are trying to see if a polygon overlaps another polygon. It's all just math, so it's all possible, but there can be a lot of considerations, so generally better to outsource that to a library that's focused on it, IMO.

Anecdotally, I had some fun working with geometry math in the Area Generator module - which takes a parent polygon and divides it up into a set of equally-spaced sub-polygons (to represent parallel beds). Also in the code for calculating the area of a polygon. Learned a lot about projections/coordinate systems in the process - and that there are a lot more considerations to it than I thought! :-)

mstenta commented 5 years ago

https://gis.stackexchange.com/questions/147829/how-to-check-for-geometry-intersection-point-in-polygon-using-openlayers3

That points to a library called turf.js: http://turfjs.org/

mstenta commented 5 years ago

Looks like turf.js offers a lot of the same features/functions as GeoPHP/GEOS! Cool!

jgaehring commented 5 years ago

@alexadamsmith Do you want to open a separate issue for this where we can track it?

jgaehring commented 5 years ago

Closing this as it should be resolved as of https://github.com/farmOS/farmOS-native/commit/03863e97df9b9b21ce5d24400bbfde39634949df

mstenta commented 5 years ago

(Transferring all issues from old repository. See https://github.com/farmOS/farmOS-native/issues/92)