Contacts CRUD #162
Contacts CRUD #162
Conversation
| @@ -260,6 +262,70 @@ export const persistAddressesMetadata = async (walletId: string, addressesMetada | |||
| await persistWalletsMetadata(walletsMetadata) | |||
| } | |||
|
|
|||
| export const persistContact = async (contactData: ContactFormData) => { | |||
There was a problem hiding this comment.
Nit: why not adding those function to a dedicated contacts.ts file?
There was a problem hiding this comment.
Good question. I chose to leave them in this file because I made contacts be part of the wallet metadata object. Since all wallet metadata functions are in this file, I decided to keep these here as well. Not a strong opinion on this.
There was a problem hiding this comment.
Ok! If you don't have a strong opinion on this, I find that having a dedicated contact.ts file makes it easier to find the functions we're looking for, and to understand the structure of the project :)
|
Functional testing: I get an error when trying to create a contact.
|
I fixed this, it was probably cause the wallet you used had old metadata stored. I updated the code to take this into consideration. |
| innerStyle?: StyleProp<ViewStyle> | ||
| hideSeparator?: boolean | ||
| rightSideContent?: ReactNode | ||
| children?: ReactNode |
There was a problem hiding this comment.
The children prop is already defined in PressableProps I believe.
There was a problem hiding this comment.
It does, but if I remove it I get this error:
Type 'ReactNode | ((state: PressableStateCallbackType) => ReactNode)' is not assignable to type 'ReactNode'.ts(2769)
| @@ -260,6 +262,70 @@ export const persistAddressesMetadata = async (walletId: string, addressesMetada | |||
| await persistWalletsMetadata(walletsMetadata) | |||
| } | |||
|
|
|||
| export const persistContact = async (contactData: ContactFormData) => { | |||
There was a problem hiding this comment.
Ok! If you don't have a strong opinion on this, I find that having a dedicated contact.ts file makes it easier to find the functions we're looking for, and to understand the structure of the project :)
| const indexOfContactWithSameId = contacts.findIndex((c: Contact) => c.id === contactData.id) | ||
|
|
||
| if (indexOfContactWithSameId < 0) throw new Error('Could not find a contact with this ID') | ||
| if (indexOfContactWithSameAddress >= 0 && indexOfContactWithSameAddress !== indexOfContactWithSameId) |
There was a problem hiding this comment.
Nit: I'd add some new lines between those conditions. It's a bit dense 'round here (also above).
| setIsDeleting(true) | ||
|
|
||
| try { | ||
| await deleteContact(params.contactId) |
There was a problem hiding this comment.
This function call should always be followed by the dispatch of contactDeletedFromPersistentStorage right? In this case, why not dispatch directly from within the deleteContact function?
| setLoading(true) | ||
|
|
||
| try { | ||
| await persistContact(formData) |
| try { | ||
| const id = await persistContact(formData) | ||
|
|
||
| dispatch(contactStoredInPersistentStorage({ ...formData, id })) |
There was a problem hiding this comment.
Here too (I won't signal the others if any)
|
I've addressed all your comments in a follow-up PR, is that a problem for you?https://github.com/alephium/mobile-wallet/pull/167/commits |
Sorry for the long PR, the feature of contacts is a big one. I will work on the remaining sub-features (like
contact sharing,selecting contact in send screens #160, etc) in a follow-up PR.