stephanritscher / Simple-Contacts

A contacts app for managing your contacts without ads.
GNU General Public License v3.0
55 stars 2 forks source link

Data loss due to upstream bug #4

Closed lfom closed 1 year ago

lfom commented 1 year ago

Hello!

I'm glad to see a fork of SimpleContacts that tries to workaround some upstream "limitations".

When I saw the version on F-Droid I installed it right away. The idea to share contacts data only among SimplMobile is great, but it also comes with certain annoying "bugs".

One of them was the crash when showing contacts with just a company name, that was just fixed. Thank you!

Another one is about deleting all contacts with the same name without any warning and no reasonable explanation... This was reported a long time ago but the issue was closed as "won't fix":

https://github.com/SimpleMobileTools/Simple-Contacts/issues/872

If you are willing to fix this too and need help, I can try to help. I don't know much Kotlin but maybe enough to point where it happens...

Regards

stephanritscher commented 1 year ago

Hi,

I just tried that out to get to a first impression myself. I think, one has to distinguish a few cases:

  1. Merge contacts is enabled
    • Deletion from contact list deletes multiple contacts without warning => Message could be improved, but behaviour is plausible since contacts are being merged into one list entry
    • Deletion from contact view deletes multiple contacts with warning (at least in English) => Message is okay IMHO and behaviour is plausible since contacts are being merged into a combined contact view
    • Deletion from contact edit view deletes exactly the edited contact => expected, nobody complains
  2. Merge contacts is disabled
    • Deletion from contact list deletes multiple contacts without warning => Behaviour is unexpected since multiple entries are shown in the contact list
    • Deletion from contact view deletes multiple contacts without warning => Behaviour is unexpected since only data from one contact is shown in contact view
    • Deletion from contact edit view deletes exactly the edited contact => expected, nobody complains

It seems to me, that deletion must behave differently if "Merge contacts" is disabled, but behaviour is okay if "Merge contacts" is enabled (which it is by default and which I personally prefer for daily use). Do you agree with this assessment?

I'd be willing to have a look at this issue if I have time, any support appreciated.

Best regards

lfom commented 1 year ago

Hello, again!

Thank you for taking time to review it. Indeed, I forgot to mention that merge contacts was not enabled, and I do agree with your considerations!

I will try to take a look at the code soon, and I will drop a line if I find anything.

Thanks again. Regards.

lfom commented 1 year ago

Hmm, I think I have found something...

https://github.com/stephanritscher/Simple-Contacts/blob/d8e34c843883a1ba2eddc908cd41419bc0ee257d/app/src/main/kotlin/com/simplemobiletools/contacts/pro/activities/ViewContactActivity.kt#L845

deleteContactFromAllSources in ViewContactActivity always calls deleteContact with deleteClones = true, I think this is what causes the deletion of duplicates when viewing a contact, regardless if Merge Contacts is enabled or not.

There is a local variable called mergeDuplicate that gets its value from the configuration, so I think the correct line yould be:

ContactsHelper(this).deleteContact(contact!!, mergeDuplicate) {

What do you think? If it fixes the problem, then it is just a matter of finding the related strings.

stephanritscher commented 1 year ago

Hi, thanks for the suggestion. I definitely would like to give it a try the next couple of days. Best regards

stephanritscher commented 1 year ago

Hi, I'm sorry it took a while, but now I tried your suggestion and it seems to work. Check out release 6.22.4.2. Best regards