Fix the fetching of exchange rates #255
Conversation
| .startAt(startAt) | ||
| .off(VALUE_CHANGE_HOOK, onValueChange) | ||
| } | ||
| .once(VALUE_CHANGE_HOOK, fullListEmitter, errorCallback) |
There was a problem hiding this comment.
Unrelated to this PR but why is VALUE_CHANGE_HOOK = 'value'? I would assume it would equal child_added.
There was a problem hiding this comment.
If you pass value it returns the whole collection/object in one go. child_added is used so that the callback is called once for each child immediately and then whenever a new child is added it's called again. There's also a child_updated, child_moved and child_removed
There was a problem hiding this comment.
Right, so I find it confusing that VALUE_CHANGE_HOOK doesn't mean return values when there is a change but instead means return all values. Maybe just me though
packages/mobile/src/firebase/saga.ts
Outdated
| } | ||
| let cancelFunction = () => {} | ||
| const listenForNewElements = (newElementsStartAt: number) => { | ||
| cancelFunction = firebase |
There was a problem hiding this comment.
Why is this called cancelFunction?
There was a problem hiding this comment.
Yeah, I was doing this wrongly. The idea was to return a function that is called when channel is canceled so you stop listening, but I was not doing it correctly, we need to call the off function in the Firebase node ref. Switched to the correct implementation
|
|
||
| return eventChannel((emit: any) => { | ||
| const emitter = (snapshot: FirebaseDatabaseTypes.DataSnapshot) => { | ||
| const singleItemEmitter = (snapshot: FirebaseDatabaseTypes.DataSnapshot) => { |
There was a problem hiding this comment.
Can we define these outside of the eventChannel? It makes it a little more difficult to follow when all these functions are nested
There was a problem hiding this comment.
We can't because it uses the emit thing that is returned in the eventChannel :/
|
Hi @gnardini @tarikbellamine Can you please let us know if we need to test anything in this task if yes can you please provide more info to test this. |
Description
We had a bug in the fetching of the exchange rates that made us fetch the whole thing every time there was an update, which is every 30 minutes. The number of elements to fetch each time depended on how much time it passed since the last use of the app, for new users and for users that didn't open the app in a couple months we would load all the history for three months.
We were also fetching the whole exchange rate history from Firebase and filtering it in memory instead of only fetching the latest three months. We tried to filter, but we were sending
1asstartAtwhich returns everything.With the current changes, we load everything since the max between the last update and three months ago and then we listen only for new items, we don't reload everything again.
Other changes
Tested
Mostly manually, by making changes and adding and modifying items in the RTDB to see that the behavior was the expected one.
Related issues
Backwards compatibility
N/A