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

feat: Manage soil intervals on backend #1100

Merged
merged 18 commits into from
Jan 29, 2024
Merged

Conversation

david-code
Copy link
Contributor

@david-code david-code commented Jan 11, 2024

Description

This PR ended up doing a few things:

  • Removing any old code related to creating site intervals that is no longer used
  • Deleting site intervals when a site/project changes preset
  • Set up the "apply_to_all" so that the interval definitions are passed from the frontend

It's basically a grab bag of fixes that I needed in order to get the frontend to work.

Checklist

  • Corresponding issue has been opened
  • New tests added

Related Issues

Related to techmatters/terraso-mobile-client#502, but will still need frontend work

David Code Howard added 6 commits January 10, 2024 15:04
Function is responsible for deleting and creating site intervals when
the user performs the following options:
1) Updates project depth preset
2) Adds site to project / creates site in project
3) Transfers sites to project

It needs to be used in the GraphQL mutations now
Copy link
Member

@paulschreiber paulschreiber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Please have @shrouxm review as well.
  2. We cannot rely on soft_delete-d items being available, as these records get removed periodically.

terraso_backend/apps/graphql/schema/sites.py Outdated Show resolved Hide resolved
intervals = []
for site in sites:
if not hasattr(site, "soil_data"):
# Not going to create an interval if soil_data doesn't exist
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Not going to create an interval if soil_data doesn't exist
# Do not create an interval if soil_data doesn't exist

terraso_backend/apps/soil_id/models/soil_data.py Outdated Show resolved Hide resolved
Copy link
Member

@shrouxm shrouxm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i haven't had time to think through all the possibilities yet to see if there's any bugs in the implementation, but i am very nervous about the amount of backend data chasing there is with this approach. i'm trying to understand the problem a bit more, in the original thread you asked the question:

"This is what is currently causing me a headache - say there is a project level interval. What do we do when the user activates the allow input switch? The project intervals aren't saved on the backend / don't have the input information if they are."

reading through the code for the update depth interval mutation again, it seems like the answer to that question would be "a depth interval object will be created on demand to store the configuration". is that wrong, & if not is there some other issue that that creates down the line?

also, re this question from this PR:

it's not obvious to me how to differentiate between "inactive" intervals linked to the old preset, and "active" intervals linked to the new preset.

I think one implementation could be change the object manager that the soil depth interval uses. It would automatically filter out any intervals that don't match the current preset in any database query.

we could implement that logic clientside as well

maybe we should do a quick call to think through this i'm feeling very 😵‍💫 about it

terraso_backend/apps/soil_id/graphql/soil_data.py Outdated Show resolved Hide resolved
@david-code david-code force-pushed the feat/manage-soil-intervals branch from 2434c8a to 36397e6 Compare January 25, 2024 19:02
@david-code david-code merged commit b2eca72 into main Jan 29, 2024
7 checks passed
@david-code david-code deleted the feat/manage-soil-intervals branch January 29, 2024 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants