Don't import cards that can't be parsed by Sabre, but log it instead.

Fix missing 'N' or 'FN' fields on import.

4 files changed, 55 insertions, 32 deletions

diff --git a/apps/contacts/ajax/contactdetails.php b/apps/contacts/ajax/contactdetails.php
index 6ba2c7384d0..6a989c795e1 100644
--- a/apps/contacts/ajax/contactdetails.php
+++ b/apps/contacts/ajax/contactdetails.php
@@ -36,8 +36,11 @@ OC_JSON::checkLoggedIn();
 OC_JSON::checkAppEnabled('contacts');
 $l=new OC_L10N('contacts');
 
-$id = $_GET['id'];
+$id = isset($_GET['id'])?$_GET['id']:null;
 $vcard = OC_Contacts_App::getContactVCard( $id );
+if(is_null($id)) {
+	bailOut($l->t('Missing ID'));
+}
 if(is_null($vcard)) {
 	bailOut($l->t('Error parsing VCard for ID: "'.$id.'"'));
 }
diff --git a/apps/contacts/js/contacts.js b/apps/contacts/js/contacts.js
index 9b1f21feab2..26206e97c06 100644
--- a/apps/contacts/js/contacts.js
+++ b/apps/contacts/js/contacts.js
@@ -236,6 +236,7 @@ Contacts={
 			delete:function() {
 				$('#contacts_deletecard').tipsy('hide');
 				$.getJSON('ajax/deletecard.php',{'id':this.id},function(jsondata){
+					console.log('Card.delete: ' + this.id);
 					if(jsondata.status == 'success'){
 						$('#leftcontent [data-id="'+jsondata.data.id+'"]').remove();
 						$('#rightcontent').data('id','');
@@ -1055,6 +1056,7 @@ $(document).ready(function(){
 	 */
 	$('#leftcontent li').live('click',function(){
 		var id = $(this).data('id');
+		console.log('Contact ' + id + ' clicked.');
 		var oldid = $('#rightcontent').data('id');
 		if(oldid != 0){
 			$('#leftcontent li[data-id="'+oldid+'"]').removeClass('active');
@@ -1094,32 +1096,32 @@ $(document).ready(function(){
 	/**
 	 * Add and insert a new contact into the list. NOTE: Deprecated
 	 */
-	$('#contacts_addcardform input[type="submit"]').live('click',function(){
-		$.post('ajax/addcontact.php',$('#contact_identity').serialize(),function(jsondata){
-			if(jsondata.status == 'success'){
-				$('#rightcontent').data('id',jsondata.data.id);
-				$('#rightcontent').html(jsondata.data.page);
-				$('#leftcontent .active').removeClass('active');
-				var item = '<li data-id="'+jsondata.data.id+'" class="active"><a href="index.php?id='+jsondata.data.id+'"  style="background: url(thumbnail.php?id='+jsondata.data.id+') no-repeat scroll 0% 0% transparent;">'+jsondata.data.name+'</a></li>';
-				var added = false;
-				$('#leftcontent ul li').each(function(){
-					if ($(this).text().toLowerCase() > jsondata.data.name.toLowerCase()) {
-						$(this).before(item).fadeIn('fast');
-						added = true;
-						return false;
-					}
-				});
-				if(!added) {
-					$('#leftcontent ul').append(item);
-				}
-			}
-			else{
-				Contacts.UI.messageBox(t('contacts', 'Error'), jsondata.data.message);
-				//alert(jsondata.data.message);
-			}
-		}, 'json');
-		return false;
-	});
+// 	$('#contacts_addcardform input[type="submit"]').live('click',function(){
+// 		$.post('ajax/addcontact.php',$('#contact_identity').serialize(),function(jsondata){
+// 			if(jsondata.status == 'success'){
+// 				$('#rightcontent').data('id',jsondata.data.id);
+// 				$('#rightcontent').html(jsondata.data.page);
+// 				$('#leftcontent .active').removeClass('active');
+// 				var item = '<li data-id="'+jsondata.data.id+'" class="active"><a href="index.php?id='+jsondata.data.id+'"  style="background: url(thumbnail.php?id='+jsondata.data.id+') no-repeat scroll 0% 0% transparent;">'+jsondata.data.name+'</a></li>';
+// 				var added = false;
+// 				$('#leftcontent ul li').each(function(){
+// 					if ($(this).text().toLowerCase() > jsondata.data.name.toLowerCase()) {
+// 						$(this).before(item).fadeIn('fast');
+// 						added = true;
+// 						return false;
+// 					}
+// 				});
+// 				if(!added) {
+// 					$('#leftcontent ul').append(item);
+// 				}
+// 			}
+// 			else{
+// 				Contacts.UI.messageBox(t('contacts', 'Error'), jsondata.data.message);
+// 				//alert(jsondata.data.message);
+// 			}
+// 		}, 'json');
+// 		return false;
+// 	});
 	
 	$('#contacts li').bind('inview', function(event, isInView, visiblePartX, visiblePartY) {
 		if (isInView) { //NOTE: I've kept all conditions for future reference ;-)
diff --git a/apps/contacts/lib/app.php b/apps/contacts/lib/app.php
index 580cc72d5eb..724d632ea5d 100644
--- a/apps/contacts/lib/app.php
+++ b/apps/contacts/lib/app.php
@@ -58,7 +58,7 @@ class OC_Contacts_App{
 	public static function getContactObject($id){
 		$card = OC_Contacts_VCard::find( $id );
 		if( $card === false ){
-			OC_JSON::error(array('data' => array( 'message' => self::$l10n->t('Contact could not be found.'))));
+			OC_JSON::error(array('data' => array( 'message' => self::$l10n->t('Contact could not be found.').' '.$id)));
 			exit();
 		}
 
diff --git a/apps/contacts/lib/vcard.php b/apps/contacts/lib/vcard.php
index b2b29433975..bb5b2da23d5 100644
--- a/apps/contacts/lib/vcard.php
+++ b/apps/contacts/lib/vcard.php
@@ -99,7 +99,7 @@ class OC_Contacts_VCard{
 	 * @brief Adds a card
 	 * @param integer $id Addressbook id
 	 * @param string $data  vCard file
-	 * @return insertid
+	 * @return insertid on success or null if card is not parseable.
 	 */
 	public static function add($id,$data){
 		$fn = null;
@@ -107,6 +107,22 @@ class OC_Contacts_VCard{
 		$card = OC_VObject::parse($data);
 		if(!is_null($card)){
 			$fn = $card->getAsString('FN');
+			if(!$fn){ // Fix missing 'FN' field.
+				$n = $card->getAsString('N');
+				if(!is_null($n)){
+					$fn = join(' ', array_reverse(array_slice(explode(';', $n), 0, 2)));
+					$card->setString('FN', $fn);
+					OC_Log::write('contacts','OC_Contacts_VCard::add. Added missing \'FN\' field: '.$fn,OC_Log::DEBUG);
+				} else {
+					$fn = 'Unknown Name';
+				}
+			}
+			$n = $card->getAsString('N');
+			if(!$n){ // Fix missing 'N' field.
+				$n = implode(';', array_reverse(array_slice(explode(' ', $fn), 0, 2))).';;;';
+				$card->setString('N', $n);
+				OC_Log::write('contacts','OC_Contacts_VCard::add. Added missing \'N\' field: '.$n,OC_Log::DEBUG);
+			}
 			$uid = $card->getAsString('UID');
 			if(is_null($uid)){
 				$card->setUID();
@@ -137,8 +153,10 @@ class OC_Contacts_VCard{
 		}
 		else{
 			// that's hard. Creating a UID and not saving it
-			$uid = self::createUID();
-			$uri = $uid.'.vcf';
+			OC_Log::write('contacts','OC_Contacts_VCard::add. Error parsing VCard: '.$data,OC_Log::ERROR);
+			return null; // Ditch cards that can't be parsed by Sabre.
+			//$uid = self::createUID();
+			//$uri = $uid.'.vcf';
 		};
 
 		$stmt = OC_DB::prepare( 'INSERT INTO *PREFIX*contacts_cards (addressbookid,fullname,carddata,uri,lastmodified) VALUES(?,?,?,?,?)' );
@@ -175,7 +193,7 @@ class OC_Contacts_VCard{
 			if($email) {
 				$fn = $email;
 			} else {
-				$fn = 'Unknown';
+				$fn = 'Unknown Name';
 			}
 			$card->addProperty('FN', $fn);
 			$data = $card->serialize();