fix: remove broken stale request check and handle network errors in autocomplete#7441
Conversation
github-project-automation
bot
moved this to 💬 To discuss and validate
in 🤳🥫 The Open Food Facts mobile app (Android & iOS)
github-actions
bot
added
✏️ Editing
monsieurtanuki
left a comment
monsieurtanuki
left a comment
There was a problem hiding this comment.
Hi @Sherley-Sonali!
The first step would be to format your code, cf. dart format .
monsieurtanuki
left a comment
monsieurtanuki
left a comment
There was a problem hiding this comment.
Hi @Sherley-Sonali!
Please have a look at my comments.
I guess the code would indeed need some refactoring. But I'm not sure you're actually fixing the initial issue.
The assumption that caching the lists in static variables isn't good enough, offline (or worse: with low connectivity), and with recurring queries.
I cannot even find that caching in the code, beyond mere local variables - would you help me?
A solution would be to store (and reuse) the lists locally, e.g. in SQFlite in a new table.
Then, when the user types in something:
- let's have a look in the local database
- if we find something in the database, we return it, while refreshing the database with the values returned by the call to the server - will be reused for the next time
- if there's nothing in the database, we also call the server, store the list and return it
What do you think of that?
| } catch (_) { | ||
| _setLoading(false); | ||
| return _SearchResults.empty(); | ||
| } |
There was a problem hiding this comment.
| } catch (_) { | |
| _setLoading(false); | |
| return _SearchResults.empty(); | |
| } | |
| } catch (_) { | |
| return _SearchResults.empty(); | |
| } finally { | |
| _setLoading(false); | |
| } |
We're through with the loading process by now, aren't we?
| } else { | ||
| return _suggestions[search] ?? _SearchResults.empty(); | ||
| } | ||
| return _suggestions[search] ?? _SearchResults.empty(); |
There was a problem hiding this comment.
| return _suggestions[search] ?? _SearchResults.empty(); | |
| return _suggestions[search]!; |
At this point _suggestions[search] is always populated, isn't it?
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #7441 +/- ##
==========================================
- Coverage 9.54% 9.10% -0.44%
==========================================
Files 325 625 +300
Lines 16411 36636 +20225
==========================================
+ Hits 1567 3337 +1770
- Misses 14844 33299 +18455 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3 participants
What
_getSuggestions.start.difference(DateTime.now()).inSecondsalways returns a negative value,so the condition
> 5never triggered. This was dead code.AutocompleteManagerin openfoodfacts-dart, this confirms the "to be confirmed" from the issue discussion.
hides and empty results are returned cleanly instead of leaving the user
with a stuck spinner.
Screenshot or video
No visual change — this is a logic fix. Spinner now correctly disappears
on network failure instead of staying indefinitely.
Fixes bug(s)
Part of