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

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

Closed
rpc-scandinavia opened this issue May 21, 2024 · 12 comments
Closed

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

rpc-scandinavia opened this issue May 21, 2024 · 12 comments

Comments

@rpc-scandinavia
Copy link

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.

@mstilkerich
Copy link
Owner

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?

@rpc-scandinavia
Copy link
Author

rpc-scandinavia commented May 22, 2024

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:[email protected]
EMAIL;TYPE=INTERNET:[email protected]
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?

@mstilkerich
Copy link
Owner

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 => '[email protected]',  ),
  '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:[email protected]
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.

@rpc-scandinavia
Copy link
Author

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
)

@rpc-scandinavia
Copy link
Author

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.

@mstilkerich
Copy link
Owner

I have pushed a fix to master.

@rpc-scandinavia
Copy link
Author

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.

@rpc-scandinavia
Copy link
Author

rpc-scandinavia commented May 31, 2024

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

@rpc-scandinavia
Copy link
Author

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 ..

mstilkerich added a commit that referenced this issue Jun 5, 2024
…ks (#470)

When a card is moved or copied from one CardDAV addressbook to another via the roundcube web interface,
the Addressbook::insert() function will receive a save_data object produced by rcmcarddav's DataConversion
class, which includes a dummy vcard, but also the original vcard object. With the recent change to use a
vcard provided by roundcube upon the import action, this dummy vcard would wrongly be added to the target
addressbook. To avoid this, use the original vcard object that is also included in the save_data when it
has been produced by rcmcarddav.
@mstilkerich
Copy link
Owner

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

@rpc-scandinavia
Copy link
Author

rpc-scandinavia commented Jun 6, 2024

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.

@mstilkerich
Copy link
Owner

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!

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

No branches or pull requests

2 participants