SVN commit 653391 by vanrijn:

- incidence->set* triggers an update to incidence->setSyncStatus.  in
  writeIncidence(), we were calling incidence->setNonKDECustomProperty()
  for X-PILOTID, which changed syncStatus right before we harvested
  syncStatus.  these lines are now reversed so that we get syncStatus as it
  should be before we alter it.  fragile thing.  =:(
- fixed in kdepim-3.5.5+ with r653382
- fixed in branches/KDE/3.5 with r653383

BUGS:144166


 M  +10 -7     icalformatimpl.cpp  


--- trunk/KDE/kdepimlibs/kcal/icalformatimpl.cpp #653390:653391
@@ -304,6 +304,16 @@
 
 void ICalFormatImpl::writeIncidence(icalcomponent *parent, Incidence *incidence, ICalTimeZones *tzlist, ICalTimeZones *tzUsedList)
 {
+  // pilot sync stuff
+// TODO: move this application-specific code to kpilot
+  if (incidence->pilotId()) {
+    // this is a fragile thing...  whenever incidence->set* is called, it
+    // triggers an update to syncStatus.  therefore, we have to pull off
+    // the sync status before we do anything else.
+    incidence->setNonKDECustomProperty("X-PILOTSTAT", QString::number(incidence->syncStatus()));
+    incidence->setNonKDECustomProperty("X-PILOTID", QString::number(incidence->pilotId()));
+  }
+
   if ( incidence->schedulingID() != incidence->uid() )
     // We need to store the UID in here. The rawSchedulingID will
     // go into the iCal UID component
@@ -311,13 +321,6 @@
   else
     incidence->removeCustomProperty( "LIBKCAL", "ID" );
 
-  // pilot sync stuff
-// TODO: move this application-specific code to kpilot
-  if (incidence->pilotId()) {
-    incidence->setNonKDECustomProperty("X-PILOTID", QString::number(incidence->pilotId()));
-    incidence->setNonKDECustomProperty("X-PILOTSTAT", QString::number(incidence->syncStatus()));
-  }
-
   writeIncidenceBase(parent, incidence);
 
   // creation date

Blef.  This is not fixed yet.  The problem is that as soon as setNonKDECustomProperties is called in writeIncidence, it marks the events dirty again.  So, the effect is that the first save works correctly, but every subsequent save marks the records with SYNCMOD again.  Working on a patch with Reinhold now...

SVN commit 653655 by vanrijn:

- per discussion with reinhold...
- we can't use setNonKDECustomProperty for X-PILOTID and X-PILOTSTAT, since
  setting them trigger updated(), which then result in changing X-PILOTSTAT
  (syncStatus).  instead of doing that, we'll simply handle the property
  reading/writing directly.
- fixed in kdepim-3.5.5+ with r653654
BUG:144166


 M  +40 -16    icalformatimpl.cpp  
 M  +56 -24    tests/testfields.cpp  


--- branches/KDE/3.5/kdepim/libkcal/icalformatimpl.cpp #653654:653655
@@ -298,11 +298,19 @@
   // pilot sync stuff
 // TODO: move this application-specific code to kpilot
   if (incidence->pilotId()) {
-    // this is a fragile thing...  whenever incidence->set* is called, it
-    // triggers an update to syncStatus.  therefore, we have to pull off
-    // the sync status before we do anything else.
-    incidence->setNonKDECustomProperty("X-PILOTSTAT", QString::number(incidence->syncStatus()));
-    incidence->setNonKDECustomProperty("X-PILOTID", QString::number(incidence->pilotId()));
+    // NOTE: we can't do setNonKDECustomProperty here because this changes
+    // data and triggers an updated() event...
+    // incidence->setNonKDECustomProperty("X-PILOTSTAT", QString::number(incidence->syncStatus()));
+    // incidence->setNonKDECustomProperty("X-PILOTID", QString::number(incidence->pilotId()));
+
+    icalproperty *p = 0;
+    p = icalproperty_new_x(QString::number(incidence->syncStatus()).utf8());
+    icalproperty_set_x_name(p,"X-PILOTSTAT");
+    icalcomponent_add_property(parent,p);
+
+    p = icalproperty_new_x(QString::number(incidence->pilotId()).utf8());
+    icalproperty_set_x_name(p,"X-PILOTID");
+    icalcomponent_add_property(parent,p);
   }
 
   if ( incidence->schedulingID() != incidence->uid() )
@@ -1386,17 +1394,6 @@
     incidence->setUid( uid );
   }
 
-  // kpilot stuff
-// TODO: move this application-specific code to kpilot
-  QString kp = incidence->nonKDECustomProperty("X-PILOTID");
-  if (!kp.isNull()) {
-    incidence->setPilotId(kp.toInt());
-  }
-  kp = incidence->nonKDECustomProperty("X-PILOTSTAT");
-  if (!kp.isNull()) {
-    incidence->setSyncStatus(kp.toInt());
-  }
-
   // Now that recurrence and exception stuff is completely set up,
   // do any backwards compatibility adjustments.
   if ( incidence->doesRecur() && mCompat )
@@ -1448,6 +1445,33 @@
     p = icalcomponent_get_next_property(parent,ICAL_ANY_PROPERTY);
   }
 
+  // kpilot stuff
+  // TODO: move this application-specific code to kpilot
+  // need to get X-PILOT* attributes out, set correct properties, and get
+  // rid of them...
+  // Pointer fun, as per libical documentation
+  // (documented in UsingLibical.txt)
+  icalproperty *next =0;
+
+  for ( p = icalcomponent_get_first_property(parent,ICAL_X_PROPERTY);
+       p != 0; 
+       p = next )
+  {
+
+    next = icalcomponent_get_next_property(parent,ICAL_X_PROPERTY);
+
+    QString value = QString::fromUtf8(icalproperty_get_x(p));
+    QString name = icalproperty_get_x_name(p);
+
+    if (name == "X-PILOTID" && !value.isEmpty()) {
+      incidenceBase->setPilotId(value.toInt());
+      icalcomponent_remove_property(parent,p);
+    } else if (name == "X-PILOTSTAT" && !value.isEmpty()) {
+      incidenceBase->setSyncStatus(value.toInt());
+      icalcomponent_remove_property(parent,p);
+    }
+  }
+
   // custom properties
   readCustomProperties(parent, incidenceBase);
 }
--- branches/KDE/3.5/kdepim/libkcal/tests/testfields.cpp #653654:653655
@@ -58,10 +58,14 @@
     return 1;
   }
 
-  QString uid = QString::fromLatin1("KOrganizer-1345486115.965");
-  Event *e = cal.event( uid );
+  // 2 tests... first uid should result in a syncStatus of 0.  second uid
+  // should have a new summary and a 1 for syncStatus.
+  QString uid1 = QString::fromLatin1("KOrganizer-1345486115.965");
+  QString uid2 = QString::fromLatin1("KOrganizer-1345486115.967");
+
+  Event *e = cal.event( uid1 );
   if (!e) {
-    kdError() << "No event " << uid << endl;
+    kdError() << "No event " << uid1 << endl;
     return 1;
   }
 
@@ -80,58 +84,86 @@
     return 1;
   }
 
-  QString newSummary = QString::fromLatin1("Mooo summary");
+  kdDebug() << "First test passed.  Able to read fields." << endl;
 
-  e->setSummary(newSummary);
   e->setSyncStatus(KCal::Incidence::SYNCNONE);
 
+  QString newSummary = QString::fromLatin1("Mooo summary");
+
+  Event *f = new Event(*e);
+
+
+  f->setUid(uid2);
+  // add event so we trigger updated()
+  cal.addEvent(f);
+
+  f->setPilotId(34567);
+  f->setSummary(newSummary);
+
+
+
   QString filew = file +".out";
-  if (!cal.save( filew ) ) {
+  // weird, yes, I know, but we have a bug right now with saving the file
+  // twice which is corrupting X-PILOTSTAT
+  if ( !cal.save( filew ) || !cal.save( filew ) ) {
     kdError() << "Can't save " << filew << endl;
     return 1;
   }
 
 
+  // now try to read the file back in and see if our changes made it
   CalendarLocal cal2( QString::fromLatin1("UTC") );
-  // now try to read the file back in and see if our changes made it to
-  // disk
   if (!cal2.load( filew ) ) {
     kdError() << "Can't load " << filew << endl;
     return 1;
   }
 
-  QFile::remove(filew);
+  QFile::remove( filew );
 
-  e = cal2.event( uid );
+  // check for uid1--should have syncStatus of 0
+  e = cal2.event( uid1 );
   if (!e) {
-    kdError() << "No event " << uid << endl;
+    kdError() << "No event for first read test" << uid1 << endl;
     return 1;
   }
 
-  kdDebug() << "Event description " << e->summary() << endl;
+  kdDebug() << "Event 1 description " << e->summary() << endl;
 
-  if (e->hasEndDate()) {
-    QDateTime d = e->dtEnd();
-    kdDebug() << "Event ends " << d << endl;
-  }
-
   if (e->pilotId()) {
-    kdDebug() << "Pilot ID = " << e->pilotId() << endl;
-    kdDebug() << "Pilot Sync Status = " << e->syncStatus() << endl;
+    kdDebug() << "First Pilot ID = " << e->pilotId() << endl;
+    kdDebug() << "First Pilot Sync Status = " << e->syncStatus() << endl;
   } else {
-    kdError() << "No Pilot ID" << endl;
+    kdError() << "No Pilot ID for first test" << endl;
     return 1;
   }
 
-  if (e->summary() != newSummary) {
-    kdError() << "Wrong summary." << endl;
+  if (e->syncStatus() != KCal::Incidence::SYNCNONE) {
+    kdError() << "Wrong Pilot sync status." << endl;
     return 1;
   }
 
-  if (e->syncStatus() != KCal::Incidence::SYNCNONE) {
-    kdError() << "Wrong Pilot sync status." << endl;
+  // now check our second event for correctness
+  f = cal2.event( uid2 );
+
+  kdDebug() << "Event 2 description " << f->summary() << endl;
+
+  if (f->summary() != newSummary) {
+    kdError() << "Wrong summary for second read test." << endl;
     return 1;
   }
 
+  if (f->pilotId()) {
+    kdDebug() << "Second Pilot ID = " << f->pilotId() << endl;
+    kdDebug() << "Second Pilot Sync Status = " << f->syncStatus() << endl;
+  } else {
+    kdError() << "No Pilot ID for second read test" << endl;
+    return 1;
+  }
+
+  if (f->syncStatus() != KCal::Incidence::SYNCMOD) {
+    kdError() << "Wrong Pilot sync status for second read test." << endl;
+    return 1;
+  }
+
   return 0;
 }