Import #426

babelouest · 2014-02-27T13:42:25Z

Pull request retry
I did a git rebase before sending the pull request, hope it looks better now.

tanghus · 2014-02-27T15:54:59Z

I did a git rebase before sending the pull request, hope it looks better now.

Sorry, but no it doesn't. Look at the number of changed files. I guess you haven't changed 313 files? ;)

I'll have a look at it tonight (CET)

babelouest · 2014-02-27T16:34:28Z

@tanghus , no, I haven't changed all those files, although most of the reported changes are ACL changes. I don't know why I did that, but I changed most of the access control from 0644 to 0755...
Can I make git to skip it when it's only ACL changes ?

tanghus · 2014-02-27T17:13:29Z

Can I make git to skip it when it's only ACL changes ?

Yes, but not github. I suggest you make a new clone of the repo, make a new branch, and take the files you've changed and manually copy them from the old clone to the new.

But wait till tomorrow; I'll see if I can sort it out.

tanghus · 2014-02-27T18:00:20Z

@babelouest Looking at your commits I figured it was the one with the WTF ? comment 257d082 ;)

tanghus · 2014-02-27T18:24:55Z

@babelouest I think I've resolved it. Please check in the Files changed tab and see if it looks correct.

babelouest · 2014-02-27T19:39:28Z

@tanghus , looks correct to me, thanks !

tanghus · 2014-03-03T17:24:02Z

@babelouest Sorry I haven't been able to review this yet. I'm a bit tight on time in the beginning of the week though. Just to let you know I haven't forgotten :)

tanghus · 2014-03-03T17:29:59Z

formats/import_vcard_yahoo_connector.xml

+	</import_entry>
+
+	<import_entry property="X-PRIMARY-PHONE" enabled="true">
+		<vcard_entry property="PHONE" type="HOME">


@babelouest Can you add more than one type here? PRIMARY indicates it should also have type PREF. Or am I wrong here?

I'm not sure I understand your question.
Here, X-PRIMARY-PHONE stands for the property name that we're looking for, to be replace by the one below (PHONE type:HOME).
If you want to convert other properties, just add an <import_entry> tag with the <vcard_entry> tag for the result.
What do you mean by PREF ? is it a VCard property ?

PREF is a standard type for the preferred property if there are more than one. Used for e.g TEL, EMAIL, URL, ADR etc. Like in TEL;TYPE=HOME,PREF:+1-919-676-9564 or TEL;TYPE=HOME;TYPE=PREF:+1-919-676-9564. Not sure if Yahoo has more than one PHONE type though?

so if it's a type, you can use this:
<vcard_entry property="PHONE" type="HOME,PREF">

Ah OK. Excellent!

tanghus · 2014-03-03T17:40:06Z

Just a few coding style issues. Full test later :)

babelouest · 2014-03-03T17:47:00Z

no problem :)

tanghus · 2014-03-04T18:19:44Z

@babelouest First test with a Google CSV:

csv_gmail/import/upload returned count: 0 even though there was 62 contacts and they were apparently recognized.
None of the contacts were parsed correctly, but all resulted in almost similar entries. See below.
All entries got a BDAY of current date.
CATEGORIES was recognized, but parsed incorrectly: * My Contacts ::: * Starred

Not in "My contacts":

BEGIN:VCARD
VERSION:3.0
X-UNKNOWN-ELEMENT;TYPE=ÿþName:
N:;;;;
X-UNKNOWN-ELEMENT;TYPE=Yomi Name:
X-PHONETIC-FIRST-NAME:
X-UNKNOWN-ELEMENT;TYPE=Additional Name Yomi:
X-PHONETIC-LAST-NAME:
NICKNAME:
X-UNKNOWN-ELEMENT;TYPE=Short Name:
X-UNKNOWN-ELEMENT;TYPE=Maiden Name:
BDAY:2014-03-04
GENDER:
X-UNKNOWN-ELEMENT;TYPE=Location:
X-UNKNOWN-ELEMENT;TYPE=Billing Information:
X-UNKNOWN-ELEMENT;TYPE=Directory Server:
X-UNKNOWN-ELEMENT;TYPE=Mileage:
X-UNKNOWN-ELEMENT;TYPE=Occupation:
X-UNKNOWN-ELEMENT;TYPE=Hobby:
X-UNKNOWN-ELEMENT;TYPE=Sensitivity:
X-UNKNOWN-ELEMENT;TYPE=Priority:
X-UNKNOWN-ELEMENT;TYPE=Subject:
NOTE:
CATEGORIES:
EMAIL1.EMAIL;TYPE=* :
FN:
UID:20140304T175950.59641b4556@localhost
REV:2014-03-04T18:00:09+00:00
PRODID:-//ownCloud//NONSGML Contacts 0.3.0.1//EN
END:VCARD

In "My contacts":

BEGIN:VCARD
VERSION:3.0
X-UNKNOWN-ELEMENT;TYPE=ÿþName:
N:;;;;
X-UNKNOWN-ELEMENT;TYPE=Yomi Name:
X-PHONETIC-FIRST-NAME:
X-UNKNOWN-ELEMENT;TYPE=Additional Name Yomi:
X-PHONETIC-LAST-NAME:
NICKNAME:
X-UNKNOWN-ELEMENT;TYPE=Short Name:
X-UNKNOWN-ELEMENT;TYPE=Maiden Name:
BDAY:2014-03-04
GENDER:
X-UNKNOWN-ELEMENT;TYPE=Location:
X-UNKNOWN-ELEMENT;TYPE=Billing Information:
X-UNKNOWN-ELEMENT;TYPE=Directory Server:
X-UNKNOWN-ELEMENT;TYPE=Mileage:
X-UNKNOWN-ELEMENT;TYPE=Occupation:
X-UNKNOWN-ELEMENT;TYPE=Hobby:
X-UNKNOWN-ELEMENT;TYPE=Sensitivity:
X-UNKNOWN-ELEMENT;TYPE=Priority:
X-UNKNOWN-ELEMENT;TYPE=Subject:
NOTE:
CATEGORIES:* My Contacts ::: * Starred
EMAIL1.EMAIL;TYPE=* Home:
FN:
UID:20140304T175950.84593cab0d@localhost
REV:2014-03-04T17:59:55+00:00
PRODID:-//ownCloud//NONSGML Contacts 0.3.0.1//EN
END:VCARD

Something to start with ;)

babelouest · 2014-03-04T18:27:45Z

OK, that's a problem indeed :p
Can you send me the csv file you used ?

tanghus · 2014-03-04T18:49:45Z

Can you send me the csv file you used ?

Done

tanghus · 2014-03-04T19:37:25Z

BTW when the contact is in the Starred group it should be added to Favorites instead.

babelouest · 2014-03-05T23:01:53Z

OK, you tried to trick me with a UTF16 file where I expect a UTF8 file ! Damn you ! ;)
This has been fixed in the code.

About the CATEGORIES issue, I see two problems there.
The fact that when there are several categories they are separated with :::and also that every category is prefixed with *, no problem, I can add a separator and remove the prefix.
But fixing when the contact is in the Starred group it should be added to Favorites instead needs more info.
Is "Favourites" a CATEGORIES value ?

tanghus · 2014-03-06T02:20:13Z

OK, you tried to trick me with a UTF16 file where I expect a UTF8 file ! Damn you ! ;)

I wasn't even aware of that. Just exported it from google contacts :)

Is "Favourites" a CATEGORIES value ?

It is kinda like the Starred group in google contacts:

For CATEGORIES you have to - besides converting them to vCard CATEGORIES - index them using the OCP\Tags lib. This is done automatically when just adding them, but you should treat Starred as Favorites in the contacts app.
In the controller that is very easy:

$tagMgr = $this->server->getTagManager()->load('contact');
$tagMgr->addToFavorites($id);

But you may wanna pass $this->server as an argument to the import class instead..?
Mentioning @jancborchardt just to be sure that Gmail Starred contacts should be filtered out and converted to favorites. And what to do with the My Contacts group? I don't quite get what google thinks is the difference between the two?

tanghus · 2014-03-06T02:24:16Z

OK, you tried to trick me with a UTF16 file where I expect a UTF8 file ! Damn you ! ;)
This has been fixed in the code.

How do you do that? I'm not sure I have come across any; at least I haven't noticed :P

babelouest · 2014-03-06T02:53:38Z

I wasn't even aware of that. Just exported it from google contacts :)

No problem, to be sure, I translate the file into UTF8 all the time now

It is kinda like the Starred group in google contacts:

Yeah but I mean, does the csv value * My Contacts ::: * Starred need to be transformed into the VCard property CATEGORIES:My Contacts,Favourites ?

tanghus · 2014-03-06T04:02:56Z

Yeah but I mean, does the csv value * My Contacts ::: * Starred need to be transformed into the VCard property CATEGORIES:My Contacts,Favourites ?

I would say it should be transformed into CATEGORIES:My Contacts plus adding the contact id to favorites using the call I quoted above.
The OCP\Tags lib (formerly OC_VCategories) was originally made for fast access access to CATEGORIES in VCARD, VEVENT, VJOURNAL etc. but I have since removed the VObject specific code from it, to make it a more general "Tagging" lib. See e.g. owncloud/core#3812
The groups in contacts app utilizes it and calendar app uses it for basic CATEGORIES editing.
For use with VObjects it requires an amount of data duplication to keep the content of the objects CATEGORIES property in sync with the index, but the code required for doing that is well made up for by the speed gained.

babelouest · 2014-03-06T15:03:14Z

I will put this in the xml format and change the code as needed to make the changes:

  <import_entry name="Group Membership" enabled="true" separator=":::" remove="*">
    <vcard_favourites value="Starred">
    </vcard_favourites>
    <vcard_entry property="CATEGORIES">
    </vcard_entry>
  </import_entry>

Then the parsing will be modified like this:

$csv_value = str_replace($remove, '', $csv_value);
$values = explode($separator, $csv_value);
foreach($values as $value) {
    if ($value == $vcard_favourites) {
        $tagMgr->addToFavorites($id);
    } else {
        // Parse normally
    }
}

tanghus · 2014-03-06T15:44:48Z

Nice!

babelouest · 2014-03-06T16:20:13Z

Or even better:

    if ($value == $vcard_favourites) {
        $property = \Sabre\VObject\Property::create("X-FAVOURITES", "yes");
        $vcard->add($property);
    } else {
        // Parse normally
    }

Then, the $tagMgr->addToFavorites($id); will be done in the importController class, right after the $addressbook->add($vcard) if the VCard has the X-FAVOURITES property set to yes.

jancborchardt · 2014-03-06T16:26:02Z

There still are a lot of merge commits and 2 "WTF?" commits. ;) Would be cool to clean the history up a bit, otherwise no one from outside knows what happened to the code.

babelouest · 2014-03-06T16:52:09Z

@jancborchardt , I agree, but how can I clean the history ? Can I make a PR with just the last commit and not the previous ones for example ? (I have googled it and couldn't find an answer...)

jancborchardt · 2014-03-06T18:49:49Z

I guess easiest here would be to make a fresh branch off master and do the changes again manually. I know it's a pain, but in this case (looking at the diff) there's also just a lot of file additions so it's not that difficult.

Is that ok?

tanghus · 2014-03-06T19:01:30Z

http://www.git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits

babelouest · 2014-03-06T22:22:23Z

@tanghus , try with the last commit for your bugs: owncloud/contacts@3b516a1
Should be fixed.

@jancborchardt , using your technique or tabghus' technique, either way I suggest I'll fix that when the new import interface will pass your tests, so it will be done once ;)
Do yo agree ?

tanghus · 2014-03-06T22:33:17Z

You mean dfba196 right? ;) I'll give it a go tomorrow.

babelouest · 2014-03-06T23:07:20Z

@tanghus , yup ;)

Author: babelouest <[email protected]>

tanghus · 2014-03-09T18:32:40Z

@babelouest @jancborchardt I made so many changes when cleaning up the code in master that this wasn't mergable anymore. A cleaned up version is now in #443

tanghus reviewed Mar 3, 2014
View reviewed changes

Import from multiple formats

9314206

Author: babelouest <[email protected]>

tanghus mentioned this pull request Mar 9, 2014

Import from multiple formats #443

Closed

tanghus closed this Mar 9, 2014

jancborchardt deleted the import branch March 13, 2014 11:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Import #426

Import #426

babelouest commented Feb 27, 2014

tanghus commented Feb 27, 2014

babelouest commented Feb 27, 2014

tanghus commented Feb 27, 2014

tanghus commented Feb 27, 2014

tanghus commented Feb 27, 2014

babelouest commented Feb 27, 2014

tanghus commented Mar 3, 2014

tanghus Mar 3, 2014

babelouest Mar 3, 2014

tanghus Mar 3, 2014

babelouest Mar 3, 2014

tanghus Mar 3, 2014

tanghus commented Mar 3, 2014

babelouest commented Mar 3, 2014

tanghus commented Mar 4, 2014

babelouest commented Mar 4, 2014

tanghus commented Mar 4, 2014

tanghus commented Mar 4, 2014

babelouest commented Mar 5, 2014

tanghus commented Mar 6, 2014

tanghus commented Mar 6, 2014

babelouest commented Mar 6, 2014

tanghus commented Mar 6, 2014

babelouest commented Mar 6, 2014

tanghus commented Mar 6, 2014

babelouest commented Mar 6, 2014

jancborchardt commented Mar 6, 2014

babelouest commented Mar 6, 2014

jancborchardt commented Mar 6, 2014

tanghus commented Mar 6, 2014

babelouest commented Mar 6, 2014

tanghus commented Mar 6, 2014

babelouest commented Mar 6, 2014

tanghus commented Mar 9, 2014

Import #426

Import #426

Conversation

babelouest commented Feb 27, 2014

tanghus commented Feb 27, 2014

babelouest commented Feb 27, 2014

tanghus commented Feb 27, 2014

tanghus commented Feb 27, 2014

tanghus commented Feb 27, 2014

babelouest commented Feb 27, 2014

tanghus commented Mar 3, 2014

tanghus Mar 3, 2014

Choose a reason for hiding this comment

babelouest Mar 3, 2014

Choose a reason for hiding this comment

tanghus Mar 3, 2014

Choose a reason for hiding this comment

babelouest Mar 3, 2014

Choose a reason for hiding this comment

tanghus Mar 3, 2014

Choose a reason for hiding this comment

tanghus commented Mar 3, 2014

babelouest commented Mar 3, 2014

tanghus commented Mar 4, 2014

babelouest commented Mar 4, 2014

tanghus commented Mar 4, 2014

tanghus commented Mar 4, 2014

babelouest commented Mar 5, 2014

tanghus commented Mar 6, 2014

tanghus commented Mar 6, 2014

babelouest commented Mar 6, 2014

tanghus commented Mar 6, 2014

babelouest commented Mar 6, 2014

tanghus commented Mar 6, 2014

babelouest commented Mar 6, 2014

jancborchardt commented Mar 6, 2014

babelouest commented Mar 6, 2014

jancborchardt commented Mar 6, 2014

tanghus commented Mar 6, 2014

babelouest commented Mar 6, 2014

tanghus commented Mar 6, 2014

babelouest commented Mar 6, 2014

tanghus commented Mar 9, 2014