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

Update Account call #1635

Closed
flexsurfer opened this issue Oct 10, 2019 · 5 comments
Closed

Update Account call #1635

flexsurfer opened this issue Oct 10, 2019 · 5 comments

Comments

@flexsurfer
Copy link
Member

flexsurfer commented Oct 10, 2019

Problem

To implement status-im/status-mobile#8446 we need ability to update account's name and colour

Implementation

accounts_updateAccount ?

Acceptance Criteria

It is possible to modify account name and color and save

@cammellos
Copy link
Contributor

@flexsurfer not sure I get this right, but looks like you already have anything you need:
https://github.com/status-im/status-react/blob/e88472a0e0246dce7283d252becb696ff0aafc13/src/status_im/multiaccounts/update/core.cljs#L43

You need to pass the whole multiaccount (it's serialized as a blob on status-go, so we don't have access to the fields), but you could just update the account you care about and pass the whole object, or am I missing something?

@flexsurfer
Copy link
Member Author

flexsurfer commented Oct 15, 2019

@cammellos why do we have accounts_saveAccounts then? it confused me

https://github.com/status-im/status-react/blob/ba34af0dd45fa256cef59ba640a7eff524dcfef4/src/status_im/wallet/accounts/core.cljs#L96

now i can see that we also save here (multiaccounts.update/multiaccount-update {:accounts (conj accounts account)

do we need accounts_saveAccounts then?

@cammellos
Copy link
Contributor

not sure, let me check what's going on with that

@cammellos
Copy link
Contributor

@flexsurfer we have a separate table to store "wallet accounts", which is what that calls is updating/inserting.

I am not still 100% sure what we need it for, but best guess is that we need it so status-go has access to the actual accounts and can ask for balances etc.

It also stores color/path etc, but as far as I can see, that data is not used, and the whole wallet accounts are denormalized under your own account (which is the one being used).

So as far as I can see updating the wallet account through a new endpoint is something that is not strictly necessary, as those fields are never used and only the "denormalized" version is used.

So what I would do for now is just leave it as it, don't update those, and we need to go back to this and either remove those fields that are never used (color etc), and use only those serialized in multiaccounts, or the opposite, don't denormalise the fieds in multiaccounts, and pull them from this table instead, as currently it's all a bit confusing.

@flexsurfer
Copy link
Member Author

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants