fix(browse): filter entities by whether they might exist in the instance #8355
Conversation
| if (filteredError || baseError) return filteredAggregations; | ||
| if (!filteredAggregations) return filteredAggregations; | ||
| if (!baseEntityAggregations) return baseEntityAggregations; | ||
| return filteredAggregations.filter((agg) => | ||
| baseEntityAggregations.some((base) => base.value === agg.value && !!base.count), | ||
| ); |
There was a problem hiding this comment.
i'm just a little confused by this logic here - so if there's an error in either of the requests, return filteredAggregations. But if there's no filteredAggregations, return it? and same for base aggregations? it looks like in the ideal case, we return filtered aggregations that exist in the base aggregations. Why would there be entity aggregations in filteredAggregations but not in baseEntityAggregations?
There was a problem hiding this comment.
Lemme walk through the logic and see if I'm missing something.
The overall idea here is to only show the filteredAggregations with 0 counts if there "could" be some if we hadn't filtered in the instance. So, we'll always drive rendering off filteredAggregations.
if (filteredError || baseError) return filteredAggregations;
We'll always just default to whatever filteredAggregations is here if there's an error. If our filteredAggregations succeeded, but the base one error'd, we'll just fallback to filteredAggregations for simplicity. In this edge case, we might end up showing the count:0 for entities not in the instance but it would be rare and only in those cases where our base query failed. It's a fallback path here.
if (!filteredAggregations) return filteredAggregations;
Again, this could be return null if clearer. Does the same thing.
if (!baseEntityAggregations) return baseEntityAggregations;
Also could be return null, does the same thing.
return filteredAggregations.filter((agg) =>
baseEntityAggregations.some((base) => base.value === agg.value && !!base.count),
);
This removes aggregations that don't have some counts in the base query. So if there's none of a certain type in the base query (like Feature Tables), we can safely remove it from the UI. But, if it is present in the base query, we can keep it even if there's a 0 count in the filtered query.
There was a problem hiding this comment.
Just pushed another commit that has the same logic but hopefully is a little bit more clear. Let me know!
There was a problem hiding this comment.
okay this is definitely helpful, and I think i was just thinking about it somewhat incorrectly as well
There was a problem hiding this comment.
No worries. Apollo is not the best for handling inter-dependent queries like this where you want to say await query(); await query(); doSomething();
joshuaeilers
force-pushed
the
je--filter-entities-if-in-instance
branch
from
de0791c
to
ccfa37a
Compare
Changes
Checklist