[Bug 41801] High level API for ContactBlocking

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=41801

--- Comment #3 from Will Thompson <will.thompson at collabora.co.uk> 2011-10-31 08:19:35 PDT ---
  Once TP_CONNECTION_FEATURE_CONTACT_BLOCKING has been prepared we can easily
  prepare TP_CONNECTION_FEATURE_CONTACT_BLOCKING on all contacts as we have all
  the information needed. Extra contact feature for free!

This commit message is wrong. One of those should be TP_CONTACT_FEATURE_…

+ * @n_contacts: the number of contacts in @contacts (must be at least 1)

but the code doesn't enforce that.


+ * @report_abusive: if %TRUE, in addition to blocking, report these contacts
as
+ * abusive to the server administrators, if supported, see
+ * #TpConnection:can-report-abusive

Better wording would be “If %TRUE, report these contacts as abusive to the
server administrators as well as blocking them. See
#TpConnection:can-report-abusive to discover whether reporting abuse is
supported. If #TpConnection:can-report-abusive is %FALSE, this parameter will
be ignored.”


+ * Expands to a call to a function that returns a #GQuark representing the
+ * "contact-blocking" feature.

Why is "contact-blocking" in quotes with a hyphen? The GQuark isn't for that
string.

+ * When this feature is prepared, the #TpConnection:can-report-abusive and
+ * #TpConnection:blocked-contacts properties of the Connection have been
+ * retrieved. The #TpContact objects from #TpConnection:blocked-contacts
+ * have been prepared with the desired features.

“When this feature is prepared, #TpConnection:blocked-contacts will contain an
up-to-date list of #TpContact<!-- -->s the user has blocked, and
#TpConnection:can-report-abusive will indicate whether abusive contacts can be
reported to the server administrator.”

I think you can put the stuff about contact features into the
#TpConnection:blocked-contacts documentation.

+      goto out;
+    }
+
+  self->priv->contact_blocking_capabilities = tp_asv_get_uint32 (properties,
+      "ContactBlockingCapabilities", &valid);
+  if (!valid)
+    {
+      DEBUG ("Connection %s doesn't have ContactBlockingCapabilities
property",
+          tp_proxy_get_object_path (self));
+    }
+
+out:

I don't really see that the goto buys us anything here.

+   * When calling tp_connection_block_contacts_async(), the contacts may be
+   * reporting as abusive to the server administrators by setting
+   * report_abusive to %TRUE.

“If this property is %TRUE, contacts may be reported as abusive to the server
administrators by setting report_abusive to %TRUE when calling
tp_connection_block_contacts_async().”

+  contacts_arr = g_ptr_array_new_with_free_func (g_object_unref);
+
+  g_hash_table_iter_init (&iter, contacts);
+  while (g_hash_table_iter_next (&iter, &key, &value))
+    {
+      TpHandle handle = GPOINTER_TO_UINT (key);
+      const gchar *id = value;
+      TpContact *contact;
+
+      contact = tp_connection_dup_contact_if_possible (self, handle, id);
+      if (contact == NULL)
+        {
+          DEBUG ("Failed to create contact %s (%d)", id, handle);
+          continue;
+        }
+
+      g_ptr_array_add (contacts_arr, contact);
+    }

There must be code elsewhere to turn an a{us} into an array of TpContacts. And
presumably also to prepare 'em all with the set of features from grabbed from a
factory for 'self'.

I think process_queued_blocked_changed leaks 'contacts'.


+  /* We are not supposed to add items to this queue while the blocked contacts
+   * have been fetched. */
+  g_assert_cmpuint (self->priv->blocked_changed_queue->length, ==, 0);

s/while/until/.

+void _tp_connection_blocked_changed_queue_free (GQueue *queue)

Coding style: add a newline.

+/**
+ * tp_contact_block_async:
+ * @self: a #TpContact
+ * @report_abusive: if %TRUE, in addition to blocking, report these contacts
as
+ * abusive to the server administrators, if supported, see
+ * #TpConnection:can-report-abusive

You copy-pasted this, so apply the same wording change as above, but also make
it singular.

+ * Convenience wrapper for tp_connection_block_contacts_async()
+ * on a single contact.

I would rather this said something like “Block communications with a contact,
optionally reporting the contact as abusive to the server administrators. To
block more than one contact at once, see tp_connection_block_contacts_async().”

+/**
+ * tp_contact_is_blocked:
+ * @self: a #TpContact
+ *
+ * <!-- -->
+
+ * Returns: the value of #TpContact:is-rblocked.

Typo.

+      /* Finish to prepare TP_CONNECTION_FEATURE_CONTACT_BLOCKING, we can
+       * prepare TP_CONTACT_FEATURE_CONTACT_BLOCKING on all contacts as we
+       * have now the list of blocked contacts. */

s/Finish to prepare/Finish preparing/; s/,/;/.


+  /* Preparing TP_CONNECTION_FEATURE_CONTACT_BLOCKING gives us
+   * TP_CONTACT_FEATURE_CONTACT_BLOCKING for free. Test that this work with
+   * and and new TpContact. */

s/work/works/; "with and and new"?

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA Contact for the bug.
You are the assignee for the bug.