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

healthequity: ignore balance-after when merging #55

Merged

Conversation

jktomer
Copy link
Contributor

@jktomer jktomer commented Jan 16, 2022

The healthequity source merges newly-downloaded transaction data into the
previously-saved file. This failed when there were multiple transactions of
the same type in a single day, because the order in which HealthEquity reports
transactions is not stable and so the running balances were not either. The
result was that past transactions would sometimes be spontaneously duplicated
in the list upon a new finance-dl run.

This change causes the merge process to ignore the "Balance After" column.
This also means that the running balance within a day may end up incorrect,
if newly-available transactions happen to be listed before
previously-available ones in the new download. There's no really good way to
prevent this except either recalculating the balance-after column ourselves
after the merge or throwing it out entirely, neither of which is proposed in
this change.

The healthequity source merges newly-downloaded transaction data into the
previously-saved file. This failed when there were multiple transactions of
the same type in a single day, because the order in which HealthEquity reports
transactions is not stable and so the running balances were not either. The
result was that past transactions would sometimes be spontaneously duplicated
in the list upon a new finance-dl run.

This change causes the merge process to ignore the "Balance After" column.
This also means that the running balance within a day may end up incorrect,
if newly-available transactions happen to be listed before
previously-available ones in the new download. There's no really good way to
prevent this except either recalculating the balance-after column ourselves
after the merge or throwing it out entirely, neither of which is proposed in
this change.
@jktomer jktomer force-pushed the healthequity-even-my-sorts-arent-stable branch from 30cf19b to 6716ce3 Compare September 2, 2022 04:46
@Zburatorul Zburatorul merged commit 76b9db7 into jbms:master Sep 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants