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

Don't create a search/pulldata database when pulldata is used over an Entity List #6471

Closed
lognaturel opened this issue Oct 23, 2024 · 6 comments
Milestone

Comments

@lognaturel
Copy link
Member

lognaturel commented Oct 23, 2024

Following #6451, pulldata over an Entity List uses the local Entity List database. However, the corresponding search/pulldata database is still being created. This increases form open time for users and leads to side effects like #6461

Currently search() does not use the Entity List database so knowing that a source CSV is for an Entity List is not a sufficient signal to skip search/pulldata database creation.

We should fix this by skipping the creation of the dynamic preload data DB if a form meets the follow two conditions:

  • Does not include search
  • If the form uses pulldata, it's only on entity lists

Notes

The big change we'll need to make for that is to persist the "flag" that an instance (media file) is an entity list so that can be determined when parsing a form. There are a lot of other reasons that storing details about media files could be useful though, so I think that's a good change to make here.

@seadowg
Copy link
Member

seadowg commented Nov 13, 2024

I'm thinking we should fix this by skipping the creation of the dynamic preload data DB if a form meets the follow two conditions:

  • Does not include search
  • If the uses pulldata, it's only on entity lists

The big change we'll need to make for that is to persist the "flag" that an instance (media file) is an entity list so that can be determined when parsing a form. There are a lot of other reasons that storing details about media files could be useful though, so I think that's a good change to make here.

Very related to this is how we then deal with search which, as @lognaturel points out, does not currently support local entities. I'm thinking we can approach it like this:

@lognaturel what are your thoughts on this plan? I think it's good to have a long term plan so we know the context surrounding fixing this issue.

@lognaturel
Copy link
Member Author

The short-term plan sounds good and I agree that we will need to know whether a form attachment is an Entity List at a time other than download for other reasons, most notably for #6425. I think that issue is higher priority than this one so maybe it can drive out storing that state.

Introduce a new XLSForm sugar to eventually replace search that pyxform translates to a normal instance('blah)...` expression

I think we already have this: select_one_from_file with a choice filter. And I think it has largely replaced search already because it's so much more comfortable to use. search is much less common than pulldata.

@seadowg
Copy link
Member

seadowg commented Nov 14, 2024

The short-term plan sounds good and I agree that we will need to know whether a form attachment is an Entity List at a time other than download for other reasons, most notably for #6425. I think that issue is higher priority than this one so maybe it can drive out storing that state.

Agreed!

I think we already have this: select_one_from_file with a choice filter. And I think it has largely replaced search already because it's so much more comfortable to use. search is much less common than pulldata.

Great point! I was forgetting that choice_filter already removes the need to deal with instance('blah')/root/item. There are other places where you do need to resort to that syntax, but search wouldn't help you there anyway.

What are your thoughts on deprecating search?

@seadowg
Copy link
Member

seadowg commented Dec 12, 2024

What are your thoughts on deprecating search?

We discussed this, and there are few things that we'd need to support more generally before getting rid of search:

  • search supports multiple languages
  • search supports showing media in select options
  • search allows specifying some static choices (like Other, Not Applicable) at the bottom of dynamic choices. We could do something like use the | operator to support specifying a join of multiple nodesets as the choice list

@seadowg
Copy link
Member

seadowg commented Dec 17, 2024

@lognaturel just checking for my own sanity here: we don't actually need to do this if we do #6552 in the same release right?

@lognaturel
Copy link
Member Author

That's right, we could potentially go straight to #6552.

@seadowg seadowg closed this as not planned Won't fix, can't repro, duplicate, stale Dec 18, 2024
@github-project-automation github-project-automation bot moved this from ready to done in ODK Collect Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: done
Development

No branches or pull requests

2 participants