[gnome-contacts/wip/nielsdg/plugins-for-form-fields: 11/11] WIP

[Niels De Graef <nielsdg src gnome org>]

commit 93a54ccd6b4e5ec780c79d8200b09813421c2b1f
Author: Niels De Graef <nielsdegraef gmail com>
Date:   Mon Dec 24 15:26:22 2018 +0100

    WIP

 src/contacts-contact-editor.vala     |  6 ++-
 src/contacts-contact-form-field.vala | 92 ++++++++++++++++++++++++++++--------
 src/contacts-contact-pane.vala       |  2 +
 src/contacts-type-descriptor.vala    | 58 ++++++++++++++++++++---
 4 files changed, 130 insertions(+), 28 deletions(-)
---
diff --git a/src/contacts-contact-editor.vala b/src/contacts-contact-editor.vala
index cdd4bd4..c2b9956 100644
--- a/src/contacts-contact-editor.vala
+++ b/src/contacts-contact-editor.vala
@@ -169,13 +169,16 @@ public class Contacts.ContactEditor : ContactForm {
   public async void save_changes () throws Error {
     for (uint i = 0; i < this.fields.get_n_items (); i++) {
       var field = this.fields.get_item (i) as EditableProperty;
-      if (field != null)
+      if (field != null) {
         yield field.save_changes ();
+        debug ("Successfully saved property '%s'", field.property_name);
+      }
     }
 
     if (name_changed ()) {
       var v = get_full_name_value ();
       yield this.contact.set_individual_property ("full-name", v);
+      debug ("Successfully saved name");
       /*XXX*/
       /* display_name_changed (v.get_string ()); */
     }
@@ -183,6 +186,7 @@ public class Contacts.ContactEditor : ContactForm {
     if (avatar_changed ()) {
       var v = get_avatar_value ();
       yield this.contact.set_individual_property ("avatar", v);
+      debug ("Successfully saved avatar");
     }
   }
 
diff --git a/src/contacts-contact-form-field.vala b/src/contacts-contact-form-field.vala
index 6419a21..1092fc3 100644
--- a/src/contacts-contact-form-field.vala
+++ b/src/contacts-contact-form-field.vala
@@ -64,8 +64,6 @@ public abstract class Contacts.PropertyField : Object {
 
 public interface Contacts.EditableProperty : PropertyField {
 
-  /* public bool dirty { get; construct set; } */
-
   /**
    * Creates a new {@link GLib.Value} from the content of this property.
    * This method is used when a new contact is created.
@@ -75,8 +73,41 @@ public interface Contacts.EditableProperty : PropertyField {
   /**
    * Saves the content of this property to the {@link Folks.Persona}. Note that
    * it is a programmer error to call this when `this.persona == null`.
+   *
+   * XXX TODO FIXME: this will time out and fail in Edsf personas if the property didn't change.
+   * Either we need to fix this in folks or make *absolutely* sure the values changed
    */
   public abstract async void save_changes () throws PropertyError;
+
+  protected bool check_if_equal (Collection<AbstractFieldDetails> old_field_details,
+                                 Collection<AbstractFieldDetails> new_field_details) {
+    // Compare FieldDetails (maybe use equal_static? using a Set)
+    foreach (var old_field_detail in old_field_details) {
+      bool got_match = false;
+      foreach (var new_field_detail in new_field_details) {
+        // Check if the values are equal
+        if (!old_field_detail.values_equal (new_field_detail))
+          continue;
+
+        // We can't use AbstractFieldDetails.parameters_equal here,
+        // since custom labels should be compared case-sensitive, while standard
+        // ones shouldn't really.
+
+        // Only compare the fields we know about => at this point only the
+        // type-related ones
+        if (!TypeDescriptor.check_type_parameters_equal (old_field_detail.parameters,
+                                                         new_field_detail.parameters))
+          continue;
+
+        got_match = true;
+      }
+
+      if (!got_match)
+        return false;
+    }
+
+    return true;
+  }
 }
 
 public class Contacts.PropertyWidget : ListBoxRow {
@@ -271,6 +302,9 @@ public class Contacts.EditableNicknameField : NicknameField, EditableProperty {
   public async void save_changes () throws PropertyError {
     assert (this.persona != null);
 
+    if (this.nickname == ((NameDetails) this.persona).nickname)
+      return;
+
     yield ((NameDetails) this.persona).change_nickname (this.nickname);
   }
 }
@@ -395,7 +429,11 @@ public class Contacts.EditableBirthdayField : BirthdayField, EditableProperty {
   public async void save_changes () throws PropertyError {
     assert (this.persona != null);
 
-    yield ((BirthdayDetails) this.persona).change_birthday (this.birthday);
+    var new_birthday = this.birthday.to_utc ();
+    if (new_birthday == ((BirthdayDetails) this.persona).birthday)
+      return;
+
+    yield ((BirthdayDetails) this.persona).change_birthday (new_birthday);
   }
 }
 
@@ -478,7 +516,8 @@ public class Contacts.EditablePhoneNrsField : PhoneNrsField, EditableProperty {
     if (this.phone_nrs.is_empty)
       return null;
 
-    var new_details = create_set ();
+    var new_details = create_new_field_details ();
+
     // Check if we only had empty phone_nrs
     if (new_details.is_empty)
       return null;
@@ -491,11 +530,16 @@ public class Contacts.EditablePhoneNrsField : PhoneNrsField, EditableProperty {
   public async void save_changes () throws PropertyError {
     assert (this.persona != null);
 
-    var new_addrs = create_set ();
-    yield ((PhoneDetails) this.persona).change_phone_numbers (new_addrs);
+    var new_phone_nrs = create_new_field_details ();
+
+    // Check if we didn't have any changes. This is a necessary step
+    // XXX explain why (timeout)
+    var old_phone_nrs = ((PhoneDetails) this.persona).phone_numbers;
+    if (!check_if_equal (old_phone_nrs, new_phone_nrs))
+      yield ((PhoneDetails) this.persona).change_phone_numbers (new_phone_nrs);
   }
 
-  private HashSet<PhoneFieldDetails>? create_set () {
+  private HashSet<PhoneFieldDetails>? create_new_field_details () {
     var new_details = new HashSet<PhoneFieldDetails> ();
     for (int i = 0; i < this.phone_nrs.size; i++) {
       if (this.phone_nrs[i] == "")
@@ -588,7 +632,7 @@ public class Contacts.EditableEmailsField : EmailsField, EditableProperty {
     if (this.emails.is_empty)
       return null;
 
-    var new_details = create_set ();
+    var new_details = create_new_field_details ();
     // Check if we only had empty emails
     if (new_details.is_empty)
       return null;
@@ -601,11 +645,14 @@ public class Contacts.EditableEmailsField : EmailsField, EditableProperty {
   public async void save_changes () throws PropertyError {
     assert (this.persona != null);
 
-    var new_addrs = create_set ();
-    yield ((EmailDetails) this.persona).change_email_addresses (new_addrs);
+    var new_emails = create_new_field_details ();
+    var old_emails = ((EmailDetails) this.persona).email_addresses;
+
+    if (!check_if_equal (old_emails, new_emails))
+      yield ((EmailDetails) this.persona).change_email_addresses (new_emails);
   }
 
-  private HashSet<EmailFieldDetails>? create_set () {
+  private HashSet<EmailFieldDetails>? create_new_field_details () {
     var new_details = new HashSet<EmailFieldDetails> ();
     for (int i = 0; i < this.emails.size; i++) {
       if (this.emails[i] != "")
@@ -690,7 +737,7 @@ public class Contacts.EditableUrlsField : UrlsField, EditableProperty {
     if (this.urls.is_empty)
       return null;
 
-    var new_details = create_set ();
+    var new_details = create_new_field_details ();
     // Check if we only had empty urls
     if (new_details.is_empty)
       return null;
@@ -703,11 +750,11 @@ public class Contacts.EditableUrlsField : UrlsField, EditableProperty {
   public async void save_changes () throws PropertyError {
     assert (this.persona != null);
 
-    var new_urls = create_set ();
+    var new_urls = create_new_field_details ();
     yield ((UrlDetails) this.persona).change_urls (new_urls);
   }
 
-  private HashSet<UrlFieldDetails>? create_set () {
+  private HashSet<UrlFieldDetails>? create_new_field_details () {
     var new_details = new HashSet<UrlFieldDetails> ();
     for (int i = 0; i < this.urls.size; i++) {
       if (this.urls[i] == "")
@@ -789,7 +836,7 @@ public class Contacts.EditableNotesField : NotesField, EditableProperty {
     if (this.notes.is_empty)
       return null;
 
-    var new_details = create_set ();
+    var new_details = create_new_field_details ();
     // Check if we only had empty addresses
     if (new_details.is_empty)
       return null;
@@ -802,11 +849,11 @@ public class Contacts.EditableNotesField : NotesField, EditableProperty {
   public async void save_changes () throws PropertyError {
     assert (this.persona != null);
 
-    var new_addrs = create_set ();
+    var new_addrs = create_new_field_details ();
     yield ((NoteDetails) this.persona).change_notes (new_addrs);
   }
 
-  private HashSet<NoteFieldDetails>? create_set () {
+  private HashSet<NoteFieldDetails>? create_new_field_details () {
     var new_details = new HashSet<NoteFieldDetails> ();
     for (int i = 0; i < this.notes.size; i++) {
       if (this.notes[i] == "")
@@ -931,7 +978,7 @@ public class Contacts.EditablePostalAddressesField : PostalAddressesField, Edita
     if (this.addresses.is_empty)
       return null;
 
-    var new_details = create_set ();
+    var new_details = create_new_field_details ();
     // Check if we only had empty addresses
     if (new_details.is_empty)
       return null;
@@ -944,11 +991,14 @@ public class Contacts.EditablePostalAddressesField : PostalAddressesField, Edita
   public async void save_changes () throws PropertyError {
     assert (this.persona != null);
 
-    var new_addrs = create_set ();
-    yield ((PostalAddressDetails) this.persona).change_postal_addresses (new_addrs);
+    var new_addrs = create_new_field_details ();
+
+    var old_addrs = ((PostalAddressDetails) this.persona).postal_addresses;
+    if (!check_if_equal (old_addrs, new_addrs))
+      yield ((PostalAddressDetails) this.persona).change_postal_addresses (new_addrs);
   }
 
-  private HashSet<PostalAddressFieldDetails>? create_set () {
+  private HashSet<PostalAddressFieldDetails>? create_new_field_details () {
     var new_details = new HashSet<PostalAddressFieldDetails> ();
     for (int i = 0; i < this.addresses.size; i++) {
       if (is_empty_postal_address (this.addresses[i]))
diff --git a/src/contacts-contact-pane.vala b/src/contacts-contact-pane.vala
index 370cdd9..52de2a6 100644
--- a/src/contacts-contact-pane.vala
+++ b/src/contacts-contact-pane.vala
@@ -226,10 +226,12 @@ public class Contacts.ContactPane : Stack {
     this.on_edit_mode = false;
     /* saving changes */
     if (!drop_changes) {
+              warning ("Editor:saving changes");
       editor.save_changes.begin ((obj, res) => {
           try {
             editor.save_changes.end (res);
           } catch (Error e) {
+              critical(e.message); // XXX
             show_message (e.message);
           }
         });
diff --git a/src/contacts-type-descriptor.vala b/src/contacts-type-descriptor.vala
index 5e1cfcc..a83fad5 100644
--- a/src/contacts-type-descriptor.vala
+++ b/src/contacts-type-descriptor.vala
@@ -106,12 +106,7 @@ public class Contacts.TypeDescriptor : Object {
     // Check whether PREF VCard "flag" is set
     bool has_pref = false;
     if (old_parameters != null) {
-      foreach (var val in old_parameters["type"]) {
-        if (val.ascii_casecmp ("PREF") == 0) {
-          has_pref = true;
-          break;
-        }
-      }
+      has_pref = TypeDescriptor.parameters_have_type_pref (old_parameters);
 
       // Copy over all parameters, execept the ones we're going to create ourselves
       foreach (var param in old_parameters.get_keys ()) {
@@ -143,6 +138,57 @@ public class Contacts.TypeDescriptor : Object {
     return new_parameters;
   }
 
+  public static bool parameters_have_type_pref (MultiMap<string, string> parameters) {
+    foreach (var val in parameters["type"])
+      if (val.ascii_casecmp ("PREF") == 0)
+        return true;
+
+    return false;
+  }
+
+  /**
+   * Checks whether the values related to a {@link TypeDescriptor} in the given
+   * parameters (as one might find in a {@link Folks.AbstractFieldDetails}) are
+   * equal.
+   *
+   * @param parameters_a: The first parameters multimap to compare
+   * @param parameters_b: The second parameters multimap to compare
+   *
+   * @return: Whether the type parameters ("type" and "PREF") are equal
+   */
+  public static bool check_type_parameters_equal (MultiMap<string, string> parameters_a,
+                                                  MultiMap<string, string> parameters_b) {
+    // First check if some "PREF" value changed
+    if (TypeDescriptor.parameters_have_type_pref (parameters_a)
+        != TypeDescriptor.parameters_have_type_pref (parameters_b))
+      return false;
+
+    // Next, check for any custom Google property labels
+    var google_label_a = Utils.get_first<string> (parameters_a[X_GOOGLE_LABEL]);
+    var google_label_b = Utils.get_first<string> (parameters_b[X_GOOGLE_LABEL]);
+    if (google_label_a != null || google_label_b != null) {
+      // Note that we do a case-sensitive comparison for custom labels
+      return google_label_a == google_label_b;
+    }
+
+    // Finally, check the type parameters
+    var types_a = new ArrayList<string>.wrap (parameters_a["type"].to_array ());
+    var types_b = new ArrayList<string>.wrap (parameters_b["type"].to_array ());
+
+    if (types_a.size != types_b.size)
+      return false;
+
+    // Now we check if types are esual. Note that we might be a bit more strict
+    // than truly necessary, but from a UI perspective they are still the same
+    types_a.sort ();
+    types_b.sort ();
+    for (int i = 0; i < types_a.size; i++)
+      if (types_a[i].ascii_casecmp (types_b[i]) != 0)
+        return false;
+
+    return true;
+  }
+
   /**
    * Converts the TypeDescriptor to a string. Should only be used for debugging.
    */