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

Add geometry/location fields to CSV importers #815

Draft
wants to merge 2 commits into
base: 3.x
Choose a base branch
from

Conversation

mstenta
Copy link
Member

@mstenta mstenta commented Mar 27, 2024

The goal of this is to add support for the following fields in CSV importers:

  • On Logs:
    • geometry
    • is_movement
  • On Assets:
    • intrinsic_geometry
    • is_location
    • is_fixed

We talked about this in some recent dev calls and I started sketching it up, so I wanted to get a draft PR opened up that we can work collaboratively on.

I have not tested any of this code yet, but I think it's functional.

Remaining todos:

  • Automated tests
  • ...

@mstenta
Copy link
Member Author

mstenta commented Mar 27, 2024

One other big question is: do we want intrinsic_geometry, is_fixed, and is_location to show on all asset types?

Technically they all have those fields, and technically it's possible for an animal to be a fixed location asset with intrinsic geometry (:stuck_out_tongue_closed_eyes:), but in practice that will almost never be the case, and including them in the CSV importers is very confusing IMO.

As it is written now, this PR adds those fields to ALL asset types. That's just the easiest way to do it. If we want to restrict it to certain types (perhaps only asset types that are locations by default, like land, structure, and water), then we would need to move the mapping definitions out of csv_asset.yml and into conditional PHP logic.

We could also punt on the question entirely and remove those fields from this PR. Then it would only add geometry and is_movement to logs, which is arguably the more widely useful feature addition. Although, I think the benefit of adding intrinsic_geometry to assets is that users could import CSVs of locations with their geometry more easily.

So, if we do need to change those to use conditional PHP logic, should we do that all in this PR, or merge the log fields first and split the asset fields out to a separate PR?

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

Successfully merging this pull request may close these issues.

None yet

1 participant