Addressee: Implemented a way to determine if the birthday has been set with a time

Review Request #128743 - Created Aug. 24, 2016 and submitted

Information
Benni Hill
kcontacts
master
Reviewers
kdepimlibs
mlaurent
  • My aim was to implement a way to determine if there has been set a explicit time with the date of birth and that the time is only exported to a vcard if it has been explicitly specified. Some vcard implementations (e.g. on Android) ignore the birthday field if it comes with a time.
    The problem here is that QDateTime (when set with a valid date) always has a valid time (midnight by default). I therefore added the boolean withTime to setBirthday() which is stored inside Addressee and can be queried with birthdayHasTime(). I also added setBirthday(QDate).
    I changed VCardTool to make use of this new methods when importing/exporting vcards.
    Probably KContacts::Field and KContacts::LDIFConverter also should be adjusted.
    To make use of this change in KAddressbook, (at least?) akonadi-contatcs has to be adjusted as well (in PersonalEditorWidget::storeContact()):
--- a/src/editor/personaleditor/personaleditorwidget.cpp
+++ b/src/editor/personaleditor/personaleditorwidget.cpp
@@ -83,12 +83,7 @@ void PersonalEditorWidget::loadContact(const KContacts::Addressee &contact)
 void PersonalEditorWidget::storeContact(KContacts::Addressee &contact)
 {
     // dates group
-    QDateTime birthday = QDateTime(mBirthdateWidget->date(), QTime(), contact.birthday().timeSpec());
-    // This is needed because the constructor above sets the time component
-    // of the QDateTime to midnight.  We want it to stay invalid.
-    birthday.setTime(QTime());
-
-    contact.setBirthday(birthday);
+    contact.setBirthday(mBirthdateWidget->date());
     Akonadi::Utils::storeCustom(contact, QStringLiteral("X-Anniversary"), mAnniversaryWidget->date().toString(Qt::ISODate));

     // family group

  • Unrelated to this changes I implemented a way in VCardTool to import/export dates without a year. Dates without a year ('--MMdd') are imported as year=-1 and during export, years <= 0 are written as '--'. (This also has to be implemented in the user inteface some where to make any use.)

Added autotests:

  • Exporting birthdays without a time.
  • Importing/Exporting dates without a year.

Issues

  • 1
  • 2
  • 0
  • 3
Description From Last Updated
What is the API/ABI policy for KContacts? This is an ABI change, when it comes to upgrading libKF5AkonadiContact without recompiling ... Friedrich W. H. Kossebau Friedrich W. H. Kossebau
Laurent Montel
Laurent Montel
Laurent Montel
Benni Hill
Benni Hill
Laurent Montel
Benni Hill
Review request changed

Status: Closed (submitted)

Change Summary:

Submitted with commit bd9f439d46ec0ca21fdac7834c7f2d5a6e8e1560 by Benjamin Löwe to branch master.
Friedrich W. H. Kossebau

   
src/addressee.h (Diff revision 3)
 
 

What is the API/ABI policy for KContacts?
This is an ABI change, when it comes to upgrading libKF5AkonadiContact without recompiling all consumers of it.

Could this perhaps be done with an overload method
void setBirthday(const QDateTime &birthday, bool withTime); instead (and a comment to merge both into one on the next ABI change occasion)?

  1. KContacts (or any other PIM library for that matter) currently does not have any API/ABI guarantees. We try not to break interfaces unless really necessary and we try to only do those changes between major releases, especially with those "low-tier" libraries.

  2. The 'KF5' in the lib name suggests some kind of stability though ;) IMHO a lib should only use the name KF5 once it is really part, but well, noone around from the marketing department :)

    But more important, please note at least @since WHATEVER VERSION in the API dox for this new method signature. Your API users will be grateful for that, incl. me :)

  3. So. This breaks an interface libkolab uses, it would be swell if it didn't. Also doing this as a separate method setBirthdayWithTime is loads more expressive IMO.

Loading...