Bug 24764 - Provide API for requesting delivery reports
Summary: Provide API for requesting delivery reports
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-qt (show other bugs)
Version: unspecified
Hardware: Other All
: medium enhancement
Assignee: Andre Moreira Magalhaes
QA Contact: Telepathy bugs list
URL: http://git.collabora.co.uk/?p=user/an...
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2009-10-27 10:46 UTC by Simon McVittie
Modified: 2009-11-02 08:11 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description Simon McVittie 2009-10-27 10:46:37 UTC
send() doesn't accept message-sending flags.

Since the only flag is currently a request for delivery reports, we propose to implement it as a Feature (TextChannel::FeatureDeliveryReports,
perhaps), since we expect that the common case is that you want delivery
reports for each message sent in a channel. If enabled, FeatureDeliveryReports
should become ready immediately (in practice there'd be no need to wait for it,
unless you're enabling other features that you *do* want to wait for, at the
same time).

If it has been requested, the channel should set the "delivery report
requested" flag on all messages sent through send().

If we add other flags in future, we can consider whether to add a 'flags' parameter to send() on a case-by-case basis.
Comment 1 Simon McVittie 2009-10-28 10:20:37 UTC
>diff --git a/TelepathyQt4/text-channel.cpp b/TelepathyQt4/text-channel.cpp
>index be82cff..ccf897d 100644
>--- a/TelepathyQt4/text-channel.cpp
>+++ b/TelepathyQt4/text-channel.cpp
>@@ -102,6 +102,7 @@ struct TELEPATHY_QT4_NO_EXPORT TextChannel::Private
>     static void introspectMessageQueue(Private *self);
>     static void introspectMessageCapabilities(Private *self);
>     static void introspectMessageSentSignal(Private *self);
>+    static void introspectMessageDeliveryReports(Private *self);
> 
>     void updateInitialMessages();
>     void updateCapabilities();

This doesn't introspect anything. Call it enableMessageDeliveryReports?

>@@ -180,6 +185,14 @@ TextChannel::Private::Private(TextChannel *parent)
>         this);
>     introspectables[FeatureMessageSentSignal] = introspectableMessageSentSignal;
> 
>+    ReadinessHelper::Introspectable introspectableMessageDeliveryReports(
>+        QSet<uint>() << 0,                                                      // makesSenseForStatuses
>+        Features() << Channel::FeatureCore,                                     // dependsOnFeatures (core)

Shouldn't it depend on FeatureMessageQueue too, since that's how you actually
*get* the reports?

>+/**
>+ * \var Feature TextChannel::FeatureMessageDeliveryReports
>+ * The messageReceived will be emitted as soon as a message of type
>+ * ChannelTextMessageTypeDeliveryReport is received.
>+ * Note that delivery report needs to be supported. To check if delivery report
>+ * is supported enable FeatureMessageCapabilities and check
>+ * deliveryReportingSupport().
>+ */

I don't like this description. How about this?

 * When enabled, delivery reports will be requested. Delivery reports are
 * received in the same way as ordinary messages, but with type
 * ChannelTextMessageTypeDeliveryReport, so enabling this feature also enables
 * FeatureMessageQueue. See the Telepathy D-Bus API Specification for examples
 * of how delivery reports are represented.
 *
 * Even if this flag is set, some protocols never have delivery reports;
 * meanwhile, on some protocols, delivery reports for failure, success or
 * both will arrive even if this flag is <em>not</em> set. To check whether
 * you can expect to get delivery reports, enable FeatureMessageCapabilities
 * and check deliveryReportingSupport().

(Hmm, perhaps this Feature should imply FeatureMessageCapabilities too...)
Comment 2 Andre Moreira Magalhaes 2009-10-30 04:54:03 UTC
(In reply to comment #1)
> >diff --git a/TelepathyQt4/text-channel.cpp b/TelepathyQt4/text-channel.cpp
> >index be82cff..ccf897d 100644
> >--- a/TelepathyQt4/text-channel.cpp
> >+++ b/TelepathyQt4/text-channel.cpp
> >@@ -102,6 +102,7 @@ struct TELEPATHY_QT4_NO_EXPORT TextChannel::Private
> >     static void introspectMessageQueue(Private *self);
> >     static void introspectMessageCapabilities(Private *self);
> >     static void introspectMessageSentSignal(Private *self);
> >+    static void introspectMessageDeliveryReports(Private *self);
> > 
> >     void updateInitialMessages();
> >     void updateCapabilities();
> 
> This doesn't introspect anything. Call it enableMessageDeliveryReports?
Done

> >@@ -180,6 +185,14 @@ TextChannel::Private::Private(TextChannel *parent)
> >         this);
> >     introspectables[FeatureMessageSentSignal] = introspectableMessageSentSignal;
> > 
> >+    ReadinessHelper::Introspectable introspectableMessageDeliveryReports(
> >+        QSet<uint>() << 0,                                                      // makesSenseForStatuses
> >+        Features() << Channel::FeatureCore,                                     // dependsOnFeatures (core)
> 
> Shouldn't it depend on FeatureMessageQueue too, since that's how you actually
> *get* the reports?
Done

> >+/**
> >+ * \var Feature TextChannel::FeatureMessageDeliveryReports
> >+ * The messageReceived will be emitted as soon as a message of type
> >+ * ChannelTextMessageTypeDeliveryReport is received.
> >+ * Note that delivery report needs to be supported. To check if delivery report
> >+ * is supported enable FeatureMessageCapabilities and check
> >+ * deliveryReportingSupport().
> >+ */
> 
> I don't like this description. How about this?
> 
>  * When enabled, delivery reports will be requested. Delivery reports are
>  * received in the same way as ordinary messages, but with type
>  * ChannelTextMessageTypeDeliveryReport, so enabling this feature also enables
>  * FeatureMessageQueue. See the Telepathy D-Bus API Specification for examples
>  * of how delivery reports are represented.
>  *
>  * Even if this flag is set, some protocols never have delivery reports;
>  * meanwhile, on some protocols, delivery reports for failure, success or
>  * both will arrive even if this flag is <em>not</em> set. To check whether
>  * you can expect to get delivery reports, enable FeatureMessageCapabilities
>  * and check deliveryReportingSupport().
Verbatim
 
> (Hmm, perhaps this Feature should imply FeatureMessageCapabilities too...)
I didn't change this as I don't have a strong opinion here. I believe enabling delivery reports is independent of knowing if delivery reports are supported.
It's like, _if_ reports are supported, enable it.
Comment 3 Andre Moreira Magalhaes 2009-11-02 08:11:14 UTC
We decided to implement this as a flag in the send method. So there is no feature anymore. Bug fixed and it will be included in next upstream version 0.2.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.