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

fix: prevent insertion of duplicate phone numbers when calling updateContact #3

Merged
merged 1 commit into from
Apr 5, 2023

Conversation

aconanlai
Copy link

@aconanlai aconanlai commented Mar 20, 2023

This fixes the issue where we'd see vCards with multiple identical phone numbers for the same contact (sometimes formatted differently, e.g. with +1).

I've traced the error back to react-native-contacts creating new phone number entries in the database.
The issue has been solved but we forked react-native-contacts in early 2019 and haven't rebased/merged since then. Relevant github issue:
morenoh149#332

The tl:dr is: previously, updateContact would expect the phone number entries to have the id attached to it so it could update that number - without an id, it would insert a new row instead. Since updateContact expects the entire contact to be passed in as an argument, contributors found that a good solution was to delete all existing numbers and then insert the new number.

This PR patches the updateContact method with these change.

To test: you can confirm the behaviour of this bug currently by adding logs before the updateContact call in react-native in LightOS (in src/lib/LightOS/Contacts.js), as well as logging the response (the library responds with the updated contact). You can see before and after editing a contact (you don't have to even make any changes, a call to updateContact will cause the bug) that the vcard exported from the dash will have an extra TEL entry. As well, the response to updateContact will show the duplicate row(s).
Then, you can rm -rf node_modules, update the branch specified in package.json from lightos to fix/number-upsert, yarn install, clear the build cache and rebuild. Upon running the same tests, you'll see that the contact always only has 1 number in the vcard and in the response from react-native-contacts.

@aconanlai aconanlai changed the title fix: number upsert fix: prevent insertion of duplicate phone numbers when calling updateContact Mar 20, 2023
Copy link

@breannamporter breannamporter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@aconanlai aconanlai merged commit 9b33ad4 into lightos Apr 5, 2023
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