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

updateContact is only adding new information on android #332

Closed
Avery246813579 opened this issue Oct 16, 2018 · 12 comments
Closed

updateContact is only adding new information on android #332

Avery246813579 opened this issue Oct 16, 2018 · 12 comments

Comments

@Avery246813579
Copy link

I am using updateContact on android v28 and whenever I call updateContact it updates the contact by just adding the new element. For example, if I update with a new phone number it will add a new phone number to the contact and keep the old one. When trying to update the contact with empty array for say the phoneNumbers, nothing is changed and all contacts would be the same. So it seems instead of deleting the old elements inside the array, it just adds new ones and keeps the old ones.

The feature works as intended on iOS.

@morenoh149
Copy link
Owner

Sounds like update behaves like a PUT on Android but not iOS. Intended beshvior is to act like a POST on both platforms. Bug.

@jamalhassouni
Copy link

@Avery246813579 same error here

@morenoh149 morenoh149 added the bug label Dec 20, 2018
@Git-Hub-Admin
Copy link
Contributor

Also running into the same issue.

@luucv
Copy link
Contributor

luucv commented Jan 18, 2019

Same here, did one of you find a fix? @Git-Hub-Admin @jamalhassouni @Avery246813579

@Git-Hub-Admin
Copy link
Contributor

Git-Hub-Admin commented Jan 18, 2019

Looking at the code in ContactsManager.java, updateContact will look for the "id" value of each of the phoneNumber/emails/postalAddress in your update argument. If the id is not provided, it will run ContentProviderOperation.newInsert instead of ContentProviderOperation.newUpdate. (which makes sense, otherwise, you cannot identify the specific number you want to update).

What this means is by calling
Contacts.updateContact( {emailAddresses: [{label: "junk", email: "mrniet+junkmail@test.com"}] } ),
You will always end up invoking newInsert because id is not provided.

What's you need to call is
Contacts.updateContact( {emailAddresses: [{id: 'the id of the email record in contact', label: "junk", email: "mrniet+junkmail@test.com"}] } ),

The id of each email/phone/address is not retrieved from getAll according to the documentation. But in practice, it is indeed retrieved. So make sure you call updateContact with the id of that phone number/email address/postal address, and it will update that specific one. I'm not sure how it works for iOS yet, and whether the documentation needs to be updated accordingly.

I am assuming that
{emailAddresses: [{id: 190, label: "" email:""}] }
will delete the emailAddress with id 190. But I have yet to test on that. Otherwise, there's no way to delete phone/email/address.

Also, will this be consistent with behaviors in iOS, does it do a complete overwrite for iOS contact as opposed to the id specified update? I'll try to read through the iOS code and update on this if noone else has answered this question by then.

@Avery246813579 @morenoh149 @jamalhassouni @luucv

Edit:
In RCTContacts.m, it looks like for iOS, it does a complete overwrite, meaning it would completely overwrite the existing phoneNumbers with the provided phoneNumbers. Therefore 'id' is not needed for iOS. Including id in the argument should not cause error for iOS either. It looks like it just gets ignored.
Therefore, I suggest an update an README to include this info.

On a side note, it looks like you can create labels in iOS with updateContact, but for Android, any non-default labels will be changed to "Others". It looks like there's some code defaulting anything else to "Other" for phone numbers in ContactProvider.java, but it's a bit too complex for me to figure out if that's what's causing it from just reading the code

@luucv
Copy link
Contributor

luucv commented Jan 18, 2019

In iOS it does indeed look like it rewrites the entire contact, since it doesn't have the 'id' for each phone number/email.

I forked this repo and adjusted the android functionality to fit my use case and be more consistent with iOS: it removes all the phoneNumbers and email entries, then adds the given entries again. I think this should happen for more fields on android but will dig into that later.

https://github.com/luucv/react-native-contacts

@morenoh149
Copy link
Owner

morenoh149 commented Jan 18, 2019

@Git-Hub-Admin So we need email address ids in order to update email addresses on a contact? Please help by submitting a PR to update the readme. Or a self contained blurb explaining how to get the desired effect. I don't want to misinterpret your findings.

We could also open an issue for making the behavior correct and consistent between platforms.

@Git-Hub-Admin
Copy link
Contributor

@Git-Hub-Admin @luucv

I think an opening a new issue would be great. I'm really new to open source stuff, so I don't quite know the general conventions of how things work.

I like @luucv 's approach of removing records then add them. It keeps things consistent between platforms. In that case, the README does not need to be updated. It does remove the possibility of update a single phone entry without having to copy over other phone entries as well, but then again,it doesn't do that in iOS either. so I think that's the best approach of addressing this problem.

So far the problems I spotted with Android in terms of update are:

  1. custom labels get overwritten to "Other",
  2. postal address update code doesn't exist. (it exists for addContact)
  3. phoneNumbers and emails insert instead of update.

I've forked the repo, and I can probably make a pull request with these issues addressed later today or tomorrow.

@morenoh149
Copy link
Owner

morenoh149 commented Jan 18, 2019

I think this issue can continue tracking the problem. I went ahead and updated the readme to note the issues.

We just need a PR fixing the issue by either making the api behave in an expected way and/or making the api behave the same on both platforms. PRs welcome.

@paintedbicycle
Copy link
Contributor

paintedbicycle commented Feb 6, 2019

Do any of these merges (PR #348 or PR #349) fix #270? Seems very similar

@morenoh149
Copy link
Owner

morenoh149 commented Feb 6, 2019

@paintedbicycle they are merged. try the latest releases. We did merge some contact array field changes (emails and urls) so it's worth checking out.

@github-actions
Copy link

This issue is stale, please provide more information about the status

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

6 participants