Bug 32267

howzat?

I can't find a bug# regarding the spec, but it looks good.

Implementation looks good, but I'd also like to see a test in which the Observer follows the pattern from Bug #27860 (with either Claim or HandleWith), and as a result, ADO isn't called *at all*; for the cases mentioned in that bug (and in the spec branch here), it seems as though that's the important thing we want.

Relatedly, it would be really great (but isn't a merge blocker for this) if you could fix Bug #27860 (document that observers can act like non-interactive approvers). Perhaps the situations Will mentions there could help to make this bug's spec rationale more concrete, too.

Okay cool, I added a test to do what bug #27860 describes, and I guess it's just as well you asked for it as I had to fix MC to not assert.

Branch updated.

+        if (approver_event_id > 0)
+        {
+            DEBUG ("Cancelling call to approvers as dispatch operation has been Claimed");
+            g_source_remove (approver_event_id);
+        }

This only seems to happen for Claim: what happens if a non-interactive approver calls HandleWith?

(In reply to comment #4)
> This only seems to happen for Claim: what happens if a non-interactive approver
> calls HandleWith?

I've added a test. It did accidentally work, but I've made it work a little more obviously. See my branch!

Spec: I think it would be good to document what's the "default" value of this property: i.e. how is MC supposed to deal with an old Observer not implementing the property.

(In reply to comment #6)
> Spec: I think it would be good to document what's the "default" value of this
> property: i.e. how is MC supposed to deal with an old Observer not implementing
> the property.

I've done this in http://git.collabora.co.uk/?p=user/cassidy/telepathy-spec;a=shortlog;h=refs/heads/delay-approver

Jonny ++ed it, so we can merge it at the same time as the implementation.

For the record, I opened bug #34964 about the implementation of DelayApprovers in TpBaseClient.

I merged the spec branch (will be in 0.21.11) and the MC one (will be in 5.7.6).