Improve upload account balance history #86

CalvinChanCan · 2024-03-02T17:24:42Z

This PR improves on the upload account balance history.

The method upload_account_balance_history only uploads the CSV file but does not update the balance in Monarch Money.

A new method parse_upload_balance_history_session is added to parse the csv file which would cause an update to the account balance history.

Another new method get_upload_balance_history_session is added to check the status of whether the parsing is still processing or completed.

Finally upload_and_parse_balance_history uses these 3 methods together to upload and parse the account balance history so that it shows up in Monarch Money.

hammem

Thanks, @CalvinChanCan ! What's the difference between using this and uploading transactions to an account? I don't understand how this will appear different to a person in the UX once it's done.

I have a bunch of inline comments about the naming of methods, ensuring it's consistent with patterns already set in the API, and ensuring you don't accidentally include your changes from #85

hammem · 2024-03-05T20:24:43Z

README.md

 - `set_budget_amount` - sets a budget's value to the given amount (date allowed, will only apply to month specified by default). A zero amount value will "unset" or "clear" the budget for the given category.
 - `create_manual_account` - creates a new manual account
- `upload_account_balance_history` - uploads account history csv file for a given account
+- `delete_account` - deletes an account by the provided account id


Please pull this branch off of your other commits in the other PR, so it doesn't accidentally introduce those changes simultaneously.

hammem · 2024-03-05T20:25:22Z

monarchmoney/monarchmoney.py

            variables=variables,
        )

+    async def delete_account(


same as above, please stack these commits atop origin/main, instead of the other branch you've created for #85

hammem · 2024-03-05T20:27:15Z

monarchmoney/monarchmoney.py


        :param account_id: The account ID to apply the history to.
-        :param csv_content: CSV representation of the balance history.
+        :param csv_content: CSV representation of the balance history. Headers: Date, Amount, and Account Name.


nit: move this description into the body of the docblock, instead of here, where it's challenging to fit.

hammem · 2024-03-05T20:27:47Z

monarchmoney/monarchmoney.py

+            response = await resp.json()
+            session_key = response["session_key"]
+            return session_key


return the entire JSON block, for consistency with the rest of the API.

I refactor this quite a bit since the initial review so I've added _upload_form_data as a way to generalize the uploading form data (I'm hoping to use this later to bulk add transactions from a csv). Anyway, this should now return the whole response.

hammem · 2024-03-05T20:28:37Z

monarchmoney/monarchmoney.py

+        Uploads and parses the balance history, updating the account balance on monarch money
+
+        :param account_id: The account ID to apply the history to.
+        :param csv_content: CSV representation of the balance history. Headers are Date, Amount, and Account Name.


This should probably be something more structured than a str? If there are specific columns and column types/formats needed, perhaps using a List[Dict[str, Any]]? Alternatively, use either Python's built-in CSV parsing library or, maybe, Pandas.

nit: move the description of the CSV format required to the body of the docblock

If we're going this far to normalize the input, I'd vote for making the list value a class (or DataClass/NamedTuple). This way the caller doesn't need to worry about what the magic key names are.

I agree that a dataclass would help the caller with the proper input. I've refactor it to use a dataclass. A second review over this would be appreciated!

Personally, I'm fine with providing the CSV as a string. This gives the user the most flexibility to use the tools of their choice to build the CSV. If we require a normalized input, then they'll have to use a CSV parser to parse the input, only for it to generate a CSV once again.

Optionally, we could provide users with a CSV builder kind of class if we want to help them along. IMO this could be an improvement made outside of this PR. That's @hammem 's call :)

Thanks for the good discussion on this one! Yeah, it's not ideal no matter which route you take. If only we knew why @monarchmoney decided to not use a GraphQL endpoint for uploading this data...

The option to go with a dataclass is great and probably something to do elsewhere.

My main concern with a raw CSV string is it makes debugging a huge pain for end users, as we can't guide or hint where things might be off.

But, I've been holding this up for a while, no need to keep holding it up for that.

Appreciate everyone's patience!

hammem · 2024-03-05T20:28:44Z

monarchmoney/monarchmoney.py

+    async def upload_and_parse_balance_history(
+        self,
+        account_id: str,
+        csv_content,


needs a typehint

To keep upload_account_balance_history, I've removed upload_and_parse_balance_history so this isn't necessary anymore.

hammem · 2024-03-05T20:29:28Z

monarchmoney/monarchmoney.py

+            session_key = response["session_key"]
+            return session_key
+
+    async def parse_upload_balance_history_session(self, session_key: str) -> dict:


nit

Suggested change

async def parse_upload_balance_history_session(self, session_key: str) -> dict:

async def initiate_upload_balance_history_session(self, session_key: str) -> dict:

hammem · 2024-03-05T20:34:24Z

monarchmoney/monarchmoney.py

+        timeout: int = 300,
+        delay: int = 10,


define these as constants, so it can be consistent across the API

hammem · 2024-03-05T20:35:41Z

monarchmoney/monarchmoney.py

+            "Web_ParseUploadBalanceHistorySession", query, variables
+        )
+
+    async def get_upload_balance_history_session(self, session_key: str):


for consistency with similar methods in this API

Suggested change

async def get_upload_balance_history_session(self, session_key: str):

async def is_upload_balance_history_complete(self, session_key: str) -> bool:

andrecloutier · 2024-03-07T01:38:46Z

monarchmoney/monarchmoney.py

+
+            is_completed = (await self.get_upload_balance_history_session(session_key))[
+                "uploadBalanceHistorySession"
+            ]["status"]


I believe this is missing a == "completed" check.

Though, I believe a better approach would be to check whether the processing is still "pending" so that this code short circuits if the upload fails.

Yes, it is missing the check! Thanks for catching that!

The possible statuses that I've found so far are: "created", "started", and "completed". After calling _initiate_upload_balance_history_session, it usually returns the "created" status. Then _is_upload_balance_history_complete can either return the "started" or "completed" statuses.

If _is_upload_balance_history_complete can return "created", it may lead to a premature completed check. Though I don't think that's likely.

@hammem , thoughts on whether we should check for "started" vs "completed"? I could see the better case being checking for "started" but worried about possible side effects that may cause.

Ah I was expecting a failed state if an invalid csv is provided. In that case, looks like the following error is thrown. Which I think is sufficient. Please disregard the suggestion!

gql.transport.exceptions.TransportQueryError: {'message': "Something went wrong while processing: ['parseBalanceHistory']

@CalvinChanCan , after looking through the flow, I think you've picked the better option. the other can introduce more complicated handling for end-users.

andrecloutier · 2024-03-07T01:49:16Z

What's the difference between using this and uploading transactions to an account?

This API allows you to set the balance history on an account irrespective of the transactions. For my use case, I use this API to sync nightly the value of my brokerage accounts with Monarch. There's no transactions to report, the value just fluctuates every day.

@CalvinChanCan - Thanks for putting this PR together! I've been meaning to contribute back some local hacks I had to account for the changes made to the API... but this PR is much more complete!

andrecloutier · 2024-03-07T02:18:54Z

README.md

 - `create_manual_account` - creates a new manual account
- `upload_account_balance_history` - uploads account history csv file for a given account
+- `delete_account` - deletes an account by the provided account id
+- `upload_and_parse_balance_history` - uploads and parses account history csv file for a given account


I'd vote for keeping the existing method rather than make a breaking change. The fact that the API now requires us to poll for the result is an implementation detail.

Thanks @andrecloutier. That's a fair point. I did a bunch of refactoring to keep the existing method upload_account_balance_history.

…o feat-upload-balance-history

…o feat-upload-balance-history # Conflicts: # monarchmoney/monarchmoney.py

…ad_balance_history_session

…e_history_complete

…across API

andrecloutier

Looks good to me! I took these changes for a test drive locally and was able to use it to update some balances in my account.

andrecloutier · 2024-03-09T21:29:50Z

monarchmoney/monarchmoney.py

+        csv_content: str,
+        timeout: int = TIMEOUT,
+        delay: int = DELAY,
+    ) -> str:


Response is a bool.

fixed. thanks for catching this!

…istory

CalvinChanCan · 2024-03-20T02:23:33Z

Hi @hammem, just wondering if I could get another review. Happy to make any further changes/feedback.

CFarzaneh · 2024-03-22T15:20:51Z

I pulled down this code to try and use it. can you please give example input? How do I use BalanceHistoryRow?

CalvinChanCan · 2024-03-23T02:35:50Z

Hi @CFarzaneh ,

You can import the class BalanceHistoryRow. With this class, you only need to provide the date (as a datetime object), an amount as a float, and optionally, can you include the account_name.

Example:

rows_list = []
row = BalanceHistoryRow(
    date=datetime.datetime(2020, 1, 1),
    amount=17.00,
    account_name='Chase Bank Account',
)
rows_list.append(row)

mm.upload_account_balance_history(account_id='1700000000000', csv_content=rows_list)

hammem · 2024-04-14T19:44:39Z

@CalvinChanCan , apologies for the delay on this. Just a couple of minor things and this is ready to go. Once you are ready, I'll merge and do a release with this as well.

monarchmoney/monarchmoney.py

hammem · 2024-04-14T19:29:17Z

monarchmoney/monarchmoney.py

        """Performs multi-factor authentication to access a Monarch Money account."""
        await self._multi_factor_authenticate(email, password, code)

+    async def _upload_form_data(self, url: str, data: FormData) -> dict:


nit: please move methods like this down to the bottom for folks that happen to read the code as an end-user, as _gql_call() and others are.

hammem · 2024-04-14T19:36:44Z

monarchmoney/monarchmoney.py

+        Uploads and parses the balance history, updating the account balance on monarch money
+
+        :param account_id: The account ID to apply the history to.
+        :param csv_content: CSV representation of the balance history. Headers are Date, Amount, and Account Name.


Thanks for the good discussion on this one! Yeah, it's not ideal no matter which route you take. If only we knew why @monarchmoney decided to not use a GraphQL endpoint for uploading this data...

The option to go with a dataclass is great and probably something to do elsewhere.

My main concern with a raw CSV string is it makes debugging a huge pain for end users, as we can't guide or hint where things might be off.

But, I've been holding this up for a while, no need to keep holding it up for that.

Appreciate everyone's patience!

hammem · 2024-04-14T19:43:25Z

monarchmoney/monarchmoney.py

+
+            is_completed = (await self.get_upload_balance_history_session(session_key))[
+                "uploadBalanceHistorySession"
+            ]["status"]


@CalvinChanCan , after looking through the flow, I think you've picked the better option. the other can introduce more complicated handling for end-users.

hammem · 2024-04-20T14:29:41Z

@CalvinChanCan , lmk if you'd like this included in the next release to PyPI!

@CalvinChanCan

merging for @CalvinChanCan

@CalvinChanCan

Merging for @CalvinChanCan

…to main-bradleyseanf

…in-bradleyseanf

CalvinChanCan added 3 commits March 2, 2024 11:42

feat: upload and parse account balance

9b8c69b

chore: fix formatting

813adca

chore: update README.md

1593b90

hammem requested changes Mar 5, 2024

View reviewed changes

andrecloutier mentioned this pull request Mar 7, 2024

upload_account_balance_history endpoint is not working as expected #87

Open

andrecloutier reviewed Mar 7, 2024

View reviewed changes

CalvinChanCan added 19 commits March 7, 2024 01:58

feat: add delete_account

9bf0010

feat: upload and parse account balance

cbff400

chore: fix formatting

1d2eb40

chore: update README.md

a4c614c

Merge remote-tracking branch 'origin/feat-upload-balance-history' int…

76ebcde

…o feat-upload-balance-history

remove delete_account from other branch

00c6760

feat: add delete_account

7ad7961

fix merge conflicts

967447b

chore: update README.md

ad421a8

remove delete_account from other branch

d6f5180

Merge remote-tracking branch 'origin/feat-upload-balance-history' int…

14beaf3

…o feat-upload-balance-history # Conflicts: # monarchmoney/monarchmoney.py

add back delete_account

1d07ae0

refactor: keep existing upload_account_balance_history

8b37717

fix: add back delete_account to readme

76d25b2

chore: rename _parse_upload_balance_history_session to _initiate_uplo…

e7acd66

…ad_balance_history_session

chore: rename get_upload_balance_history_session to _is_upload_balanc…

1fe7987

…e_history_complete

refactor: define constants for delay and timeout to use consistently …

ee959a3

…across API

chore: move description into body of docblock

7f68c9c

chore: lint

44711ad

andrecloutier approved these changes Mar 9, 2024

View reviewed changes

andrecloutier reviewed Mar 9, 2024

View reviewed changes

fix: update return type from str to bool for upload_account_balance_h…

4e2d624

…istory

CalvinChanCan added 2 commits March 10, 2024 01:17

refactor: replace csv_content str with dataclass

53b6abc

chore: add docstring

86a0311

CalvinChanCan requested a review from hammem March 26, 2024 03:10

hammem linked an issue Apr 14, 2024 that may be closed by this pull request

upload_account_balance_history endpoint is not working as expected #87

Open

hammem requested changes Apr 14, 2024

View reviewed changes

hammem and others added 14 commits January 19, 2025 11:03

Update monarchmoney/monarchmoney.py

ee41dc6

merging for @CalvinChanCan

Update monarchmoney/monarchmoney.py

9394a97

Merging for @CalvinChanCan

fix: persist long-lived session tokens (Fixes hammem#139)

508f9bf

Update README.md

2d4f1b4

Create assets folder

89a9ce9

Update readme.Md with auth correction notice

b6bbf50

Merge branch 'hammem:main' into fix/session-persistence-#139

d97de7f

Fix get_budgets query for legacy goals removal

34e440d

Merge remote-tracking branch 'origin/fix/session-persistence-#139' in…

b6732d6

…to main-bradleyseanf

Merge remote-tracking branch 'origin/fix/get_budgets()-query' into ma…

a1703a3

…in-bradleyseanf

updating gql endpoint

d317b9a

Release v1.0.0

3989f3e

Update README

147b1aa

Merge branch 'community-dev' into feat-upload-balance-history

88e3dc7

CalvinChanCan mentioned this pull request Jan 17, 2026

Improve upload account balance history bradleyseanf/monarchmoneycommunity#2

Merged

	async def parse_upload_balance_history_session(self, session_key: str) -> dict:
	async def initiate_upload_balance_history_session(self, session_key: str) -> dict:

	async def get_upload_balance_history_session(self, session_key: str):
	async def is_upload_balance_history_complete(self, session_key: str) -> bool:

Uh oh!

Improve upload account balance history #86

Are you sure you want to change the base?

Improve upload account balance history #86

Uh oh!

Conversation

CalvinChanCan commented Mar 2, 2024

Uh oh!

hammem left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrecloutier commented Mar 7, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrecloutier left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CalvinChanCan commented Mar 20, 2024

Uh oh!

CFarzaneh commented Mar 22, 2024

Uh oh!

CalvinChanCan commented Mar 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CalvinChanCan commented Mar 23, 2024 •

edited

Loading