UsedForField widget on sighting

[sandreae]

Implemented

• UsedFor model and associated queries
• UsedForField widget for adding existing "used for" tags to a sighting, or creating new ones and directly adding them.
• makes use of InfiniteScrollList and InfiniteDedupTagsList which both handle making paginated requests, showing the results in a list, and refreshing the query on scroll to bottom. There is a lot of code duplication in these components (because of the (ugly) dedup logic required when querying all uses, see below for details) but I couldn't see a better way so far.

Possible Improvements

• design can definitely be improved
• maybe add a button for submitting new uses, rather than clicking the top level submit button (which then closes edit mode)
• think of a way to not require the dedup logic when querying all uses. One way is as described in the comments below, to change our schema so that "uses" are related to from a sighting using a join document, rather than themselves containing the relation to the sighting. Then we could query for all uses documents when we want to display all tags, and query for joins when we want to show uses for one sighting. This is all optional though, and depends on how much we hate the de-duplication and/or if this same challenge occurs elsewhere

Screen_recording_20231019_161022.webm

(this vid is slightly out of date as the autocomplete widget is no longer used for text input, otherwise it's the same)

[adzialocha]

This is a bit tricky, especially when the pages contain already many existing items it will look as if nothing happened.

I know that this is a hack because of our data structure but maybe there's a way where we don't need it? I can think of changing the schema itself (similar to local names) or an UI which doesn't try to show all uses (more like a search)

[sandreae]

In the next line I account for the situation you describe by re-requesting items until we get a certain amount of unique uses. (still a hack on a hack)...

meli/packages/app/lib/ui/widgets/pagination_used_for_tags_list.dart

Lines 132 to 137 in ac0ab64

	// We want to keep fetching more results (while there are any) until
	// there are a minimum of 20. This is done
	if (uniqueUses.length < 20 && data.hasNextPage) {
	fetchMore!(opts);
	return this._loading();
	}

But yeh, totally agree that if we re-think the data type design a bit we could really make this nicer.

I think the a nice way of modelling this would be to move the sighting relation field into it's own "join" document:

BeeAttributeUsedFor {
  description: str
}

BeeAttributeUsedForAssociation {
  attribute: relation(bee_attribute_used_for)
  sighting: relation(bee_sightning)
}

This way we get;

• better re-use of UsedFor attributes
• nice aggregates over UsedFor and attributes and any associations for each sighting (totalCount)
• familiar pattern in the app for querying, displaying, auto-completing attributes (no dedup needed)
• if you delete an "association" it doesn't actually delete the attribute

[sandreae]

Any change like this we make we would want to do for the other attributes (location).

[sandreae]

We end up with even more documents and schema 🤣 but maybe that's just unavoidable with any kind of meaningful application.

[sandreae]

Alternatively we do it all like local_name (relation list with empty list representing no value) and add attribute fields for every possible attribute to bee sighting.

Here we also get all the nice repeatable behaviour around the attributes but with fewer schema (just a big sighting). But big negative is that everyone would be editing the same attribute list, so nasty collisions might occur.

[adzialocha]

What do you think of adding the join document then? Seems quite worth it to me.

It's cool! I wouldn't introduce it just for "Used for" maybe? Or is there a reason this data behaves differently than "local name"? Maybe use the same pattern for both?

[sandreae]

I think the difference between "local_name" and "used_for" is that with the former we are only wanting one value per sighting, whereas with the later we want a list.

[sandreae]

Of course, "local_name" is actually a list 🙃 but in terms of how we allow users to interact with the data, this is the difference.

[sandreae]

Is this a fundamental difference wrt this conversation about the data design?... I'm not actually sure...

[sandreae]

We did talk about allowing "local_name" to be a list at one point, as different people might know the same bee by different local names.

[adzialocha]

Thank you for this PR, this one was really tricky!

I've just tested this on my app and it almost works. This is what I've observed:

1. Whenever I look at it for the first time it is unclear what each of these fields stand for and what I can do with them. It's also unclear where the input field is to add something.

My suggestion would be to a) add titles to each of these three areas and b) introduce a "Create" button, similar to what we have with the "Notes" widget?

2. Not clear that one can scroll down the field, can we add a scrollbar here?

3. Error in GraphQL query?

Not sure what it means but I receive this here:

E/flutter ( 9891): [ERROR:flutter/runtime/dart_vm_initializer.cc(41)] Unhandled Exception: Exception: Query is not refetch safe
E/flutter ( 9891): #0 ObservableQuery.refetch (package:graphql/src/core/observable_query.dart:142:5)
E/flutter ( 9891): #1 _UsedForFieldState._createUse (package:app/ui/widgets/used_for_field.dart:63:26)
E/flutter ( 9891):
E/flutter ( 9891): #2 _UsedForFieldState._onTagClick (package:app/ui/widgets/used_for_field.dart:138:5)
E/flutter ( 9891):
E/flutter ( 9891):

Otherwise it looks fine to me! Could definitely need a little bit of design touches (less padding etc.) but don't think it's much left to make it work already.

One way is as described in the #48 (comment), to change our schema so that "uses" are related to from a sighting using a join document, rather than themselves containing the relation to the sighting

I like that! Should we go for it? It could really help with the aggregations as well ..

[sandreae]

Thanks @adzialocha, nice suggestions. I made most of those adjustments and refactored everything into smaller widgets, all much nicer now.

Whenever I look at it for the first time it is unclear what each of these fields stand for and what I can do with them. It's also unclear where the input field is to add something.

My suggestion would be to a) add titles to each of these three areas and b) introduce a "Create" button, similar to what we have with the "Notes" widget?

Yeh, I did all this, and improved the messages one sees when there are no entries in the lists.

Not clear that one can scroll down the field, can we add a scrollbar here?

Yip, added.

Error in GraphQL query?

I have actually seen this message previously, but then it suddenly stopped... Haven't seen it at all since doing a refactor of the widgets, so maybe that helped somehow... Do you still see it now?

Could definitely need a little bit of design touches (less padding etc.)

I spent a good 20mins trying to reduce the padding but can't for the life of me figure out where it's coming from 😭

One way is as described in the #48 (comment), to change our schema so that "uses" are related to from a sighting using a join document, rather than themselves containing the relation to the sighting

I like that! Should we go for it? It could really help with the aggregations as well ..

I will look more into this, but want to have a better idea what is involved in aggregating over uses per species as well. I think if we make the suggested change here, then we will still need to dedup in that case anyway..... Let me explore more in following steps.

[adzialocha]

Great, thank you for the adjustments @sandreae! Much better now, especially hiding the UI when being in "read" mode is nice. This is good to merge from my side.