mstilkerich/rcmcarddav

Importing from .vcf file does not include the "note"

rpc-scandinavia opened this issue · 12 comments

RCM CardDAV does not import "note" from .vcf files.
I found this old thread, where the issue was fixed when saving contacts:

#124

Test A:

  1. Get a .vcf file with "note" field.
  2. Import in RoundCube.
  3. The notes are missing in RoundCube and in the Radicale disk file.

Test B:

  1. Get a .vcf file with "note" field (the same file as in test A).
  2. Import in Radicale Web Interface
  3. The notes are there in RoundCube and in the Radicale disk file.

Test C:

  1. After test B, export the contacts.
  2. The notes are there in the exported .vcf file

I have:

  • Radicale CalDAV / CardDAV 3.2.0
  • RcmCardDav 5.1.0

PS: I can edit the contact in RoundCube and add a note, but not import a contact with note.

Hello,

the vcard import is done by roundcube itself, not by this plugin. Could you please check the behavior when importing to roundcube’s builtin addressbook to confirm?

Sure, thank you.
It does import "note" and "bday" when importing into "Personal addresses" in RoundCube.

This is my test .vcf file:

BEGIN:VCARD
VERSION:4.0
PRODID:-//Kopano//libicalmapi 11.0.2//EN
N:Testesen;Test;Håkon
FN:Test Håkon Testesen
ORG:NDK
TEL;TYPE=HOME:12 34 56 78
TEL;TYPE=MOBILE:23 45 67 89
TEL;TYPE=WORK:01 02 03 04
ADR;TYPE=HOME:;;Kongeveien 148, Xkjøping;Århus;;8000;DK
EMAIL;TYPE=INTERNET,PREF:test@test.dk
EMAIL;TYPE=INTERNET:test@test.no
UID:040000008200E00074C5B7101A82E00800000000806718EEF63BD801000000000000000001000000A275A1B6CC9A4368AEA4A9FF4ED9C5C9
URL:https://itdd.dk/
NOTE:This is a note that will not always import
BDAY:1972-10-26T23:00:00Z
REV:2024-05-15T22:33:01Z
END:VCARD

Import into "Personal addresses" in RoundCube:

RoundCube import

Import into CardDAV addressbook in RoundCube:

RcmCardDAV import

the vcard import is done by roundcube itself, not by this plugin. Could you please check the behavior when importing to roundcube’s builtin addressbook to confirm?

Thanks, I can reproduce it. For some reason, roundcube's vcard conversion yields the note as an array (with a single string member), where rcmcarddav expects a simple string value.

$save_data = array (
  'name' => 'Test Card',
  'firstname' => 'Test',
  'surname' => 'Test',
  'middlename' => 'Test',
  'prefix' => 'Test',
  'phone:other' =>  array (    0 => '1234',  ),
  'notes' =>  array (    0 => 'Leitung erreichbar 8-13 Uhr, Mo+Fr zusätzlich 14-16 Uhr',  ),
  'email:other' =>  array (  0 => 'foo@bar.de',  ),
  'vcard' => 'BEGIN:VCARD
VERSION:3.0
FN:Test Card
N:Test;Test;Test;Test;
NOTE:Leitung erreichbar 8-13 Uhr, Mo+Fr zusätzlich 14-16 Uhr
EMAIL:foo@bar.de
TEL:1234
END:VCARD',
)

There should also be an error message in carddav.log:

[5 ERR] save data notes must be stringArray

I am not sure whether my expectation is wrong or whether this is an issue of the roundcube vcard conversion, roundcube contacts anyway seems to be able to correctly process the result. More interestingly, however, we can see that roundcube passes along the original vcard as well. So I try to ignore the conversion result coming from roundcube in case a vcard is provided, and instead use sabre/vobject to parse and store the vcard to the CardDAV server mostly as is. This will result in a more consistent experience regarding interpretation of the vcard.

Super. I however missed that log file, but sure enough - the error appear for both the "notes" and the "bday":

[23-May-2024 20:04:45 +0200]: <3hoo8jn9> [5 ERR] save data birthday must be stringArray
(
    [0] => 1972-10-26T23:00:00Z
)
 
[23-May-2024 20:04:45 +0200]: <3hoo8jn9> [5 ERR] save data notes must be stringArray
(
    [0] => This is a note that will not always import
)

That should cover other properties as well, as long as the VCard is passed along.
I do not know PHP that well, but perhaps use a function to get/convert the propety value, and test for type like this C# ish thing:

switch (propertyVar) {
    case String[] propertyVarStringArray:
        ........
        break;
    case String propertyVarString:
        ........
        break
}

Perhaps you can have two functions you call after what YOU want/expect.
One that return a string array, and when the source is a string it returns an array with one element.
And one that return a string, and when the source is a string array, it returns the first element.

@mstilkerich : So I try to ignore the conversion result coming from roundcube in case a vcard is provided, and instead use sabre/vobject to parse and store the vcard to the CardDAV server mostly as is. This will result in a more consistent experience regarding interpretation of the vcard.

I have pushed a fix to master.

I have pushed a fix to master.

I tested it on my server, and it works with my test contact and several other contacts, with notes, that I exported from Kopano.

Nice job, thank you.

Bad news @mstilkerich, I have testet the fix, and found a unintentional bug.

  1. Drag contact from one address book to another (both are CardDAV address books)

When I perform a drag and drop operation with the version 5.1.0 copies of Addressbook.php and DataConversion.php, the contact is moved, and all data are intact.

However when I perform the same drag and drop operation with the modified copies of Addressbook.php and DataConversion.php, the contact is removed from the original address book, and the target address book gets en empty Dummy contact.

Before drag and drop with version 1.5.0 files (just imported without notes :-(
Ok, before drag

After drag and drop with version 1.5.0 files:
Ok, after drag

Before drag and drop with modified files (just imported with notes :-)
Error, before drag

After drag and drop with modified files:
Error, after drag

The Dummy contact look like this:

$ cat 97f5df4b-616a-4b55-aa79-226a1c049a67.vcf
BEGIN:VCARD
VERSION:3.0
UID:97f5df4b-616a-4b55-aa79-226a1c049a67
FN:Dummy
N:;;;;
END:VCARD

I have a suggestion to a fix.
In DataConversion.php make the EMPTY_VCF constant public:

public const EMPTY_VCF =

And in Addressbook.php change your if statement:

if ((isset($save_data['vcard'])) && ($save_data['vcard'] != DataConversion::EMPTY_VCF)) {

Then I can both import and move contacts, with notes.
I will test some more.

And yes, I had to DuckDuckGo the :: reference part, I am not a PHP programmer. in C# you just use ..

Thanks for testing this, I have added a fix for the regression as well.

I think it is working now.
I can import address with note and birthday.
I can move address with note and birthday.

At first it did not work and I got an "Imported 0 contacts" in RoundCube.
I thought that there might be some leftover data in the RoundCube database, but that was not it.
Then I discovered that my Radicale address books contained a lot of orphaned contacts, so I had to manually delete all files in the Radicale data directory, with had the VCF files UID value in its file name, then I could import my test contact again.

For the DuckDuckGo records, those were the sequel queries I executed on the MariaDB database, to delete a user in RoundCube mail (it deletes a user in the tables including your RCM CardDAV tables):

// Create the delete queries.
String[] queries = [
	// RCM CardDAV plugin.
	// user_id -> carddav_accounts.user_id
	// carddav_accounts.user_id -> carddav_addressbooks.account_id
	// carddav_addressbooks.id -> carddav_[groups|contacts|xsubtypes].abook_id
	//
	$"DELETE FROM carddav_group_user WHERE group_id IN (SELECT id FROM carddav_groups WHERE abook_id IN (SELECT id FROM carddav_addressbooks WHERE account_id IN (SELECT id FROM carddav_accounts WHERE user_id = {userId})));",
	$"DELETE FROM carddav_groups WHERE abook_id IN (SELECT id FROM carddav_addressbooks WHERE account_id IN (SELECT id FROM carddav_accounts WHERE user_id = {userId}));",
	$"DELETE FROM carddav_contacts WHERE abook_id IN (SELECT id FROM carddav_addressbooks WHERE account_id IN (SELECT id FROM carddav_accounts WHERE user_id = {userId}));",
	$"DELETE FROM carddav_xsubtypes WHERE abook_id IN (SELECT id FROM carddav_addressbooks WHERE account_id IN (SELECT id FROM carddav_accounts WHERE user_id = {userId}));",
	$"DELETE FROM carddav_addressbooks WHERE account_id IN (SELECT id FROM carddav_accounts WHERE user_id = {userId});",
	$"DELETE FROM carddav_accounts WHERE user_id = {userId};",

	// RoundCube Mail.
	$"DELETE FROM cache_shared WHERE cache_key IN (SELECT cache_key FROM cache WHERE user_id = {userId});",
	$"DELETE FROM contactgroupmembers WHERE contactgroup_id IN (SELECT contactgroup_id FROM contactgroups WHERE user_id = {userId});",
	$"DELETE FROM cache WHERE user_id = {userId};",
	$"DELETE FROM cache_index WHERE user_id = {userId};",
	$"DELETE FROM cache_messages WHERE user_id = {userId};",
	$"DELETE FROM cache_thread WHERE user_id = {userId};",
	$"DELETE FROM collected_addresses WHERE user_id = {userId};",
	$"DELETE FROM contactgroups WHERE user_id = {userId};",
	$"DELETE FROM contacts WHERE user_id = {userId};",
	$"DELETE FROM identities WHERE user_id = {userId};",
	$"DELETE FROM dictionary WHERE user_id = {userId};",
	$"DELETE FROM filestore WHERE user_id = {userId};",
	$"DELETE FROM responses WHERE user_id = {userId};",
	$"DELETE FROM searches WHERE user_id = {userId};",
	$"DELETE FROM users WHERE user_id = {userId};"
];

Note that the userId is a number, first looked up in the users table.

Ok, I‘ll close this then. The UID is supposed to be unique, so a carddav server should not accept two cards with the same UID. So what you see is intended behavior. The filename should not be an issue.

Thanks again for reporting this issue and testing the fixes!