Merge pull request #1696 from Bilb/remove-delivery-receipt

Remove delivery receipt

background.html

@@ -118,7 +118,6 @@
 
   <script type='text/javascript' src='js/focus_listener.js'></script>
   <script type='text/javascript' src='js/notifications.js'></script>
-  <script type='text/javascript' src='js/delivery_receipts.js'></script>
   <script type='text/javascript' src='js/read_receipts.js'></script>
   <script type='text/javascript' src='js/read_syncs.js'></script>
   <script type='text/javascript' src='js/expiring_messages.js'></script>

background_test.html

@@ -123,7 +123,6 @@
 
   <script type='text/javascript' src='js/focus_listener.js'></script>
   <script type='text/javascript' src='js/notifications.js'></script>
-  <script type='text/javascript' src='js/delivery_receipts.js'></script>
   <script type='text/javascript' src='js/read_receipts.js'></script>
   <script type='text/javascript' src='js/read_syncs.js'></script>
   <script type='text/javascript' src='js/expiring_messages.js'></script>

js/background.js

@@ -259,21 +259,12 @@
     window.log.info(`Cleanup: Found ${messagesForCleanup.length} messages for cleanup`);
     await Promise.all(
       messagesForCleanup.map(async message => {
-        const delivered = message.get('delivered');
         const sentAt = message.get('sent_at');
-        const expirationStartTimestamp = message.get('expirationStartTimestamp');
 
         if (message.hasErrors()) {
           return;
         }
 
-        if (delivered) {
-          window.log.info(`Cleanup: Starting timer for delivered message ${sentAt}`);
-          message.set('expirationStartTimestamp', expirationStartTimestamp || sentAt);
-          await message.setToExpire();
-          return;
-        }
-
         window.log.info(`Cleanup: Deleting unsent message ${sentAt}`);
         await window.Signal.Data.removeMessage(message.id);
       })

js/delivery_receipts.js

@@ -1,104 +0,0 @@
-/* global
-  Backbone,
-  Whisper,
-  getMessageController,
-  _,
-*/
-
-/* eslint-disable more/no-then */
-
-// eslint-disable-next-line func-names
-(function() {
-  'use strict';
-
-  window.Whisper = window.Whisper || {};
-
-  Whisper.DeliveryReceipts = new (Backbone.Collection.extend({
-    forMessage(conversation, message) {
-      let recipients;
-      if (conversation.isPrivate()) {
-        recipients = [conversation.id];
-      } else {
-        recipients = conversation.get('members') || [];
-      }
-      const receipts = this.filter(
-        receipt =>
-          receipt.get('timestamp') === message.get('sent_at') &&
-          recipients.indexOf(receipt.get('source')) > -1
-      );
-      this.remove(receipts);
-      return receipts;
-    },
-    async getTargetMessage(originalSource, messages) {
-      if (messages.length === 0) {
-        return null;
-      }
-
-      const message = messages.find(
-        item => !item.isIncoming() && originalSource === item.get('conversationId')
-      );
-      if (message) {
-        return message;
-      }
-
-      const groups = await window.Signal.Data.getAllGroupsInvolvingId(originalSource);
-
-      const ids = groups.pluck('id');
-      ids.push(originalSource);
-
-      const target = messages.find(
-        item => !item.isIncoming() && _.contains(ids, item.get('conversationId'))
-      );
-      if (!target) {
-        return null;
-      }
-
-      return getMessageController().register(target.id, target);
-    },
-    async onReceipt(receipt) {
-      try {
-        const messages = await window.Signal.Data.getMessagesBySentAt(receipt.get('timestamp'));
-
-        const message = await this.getTargetMessage(receipt.get('source'), messages);
-        if (!message) {
-          window.log.info(
-            'No message for delivery receipt',
-            receipt.get('source'),
-            receipt.get('timestamp')
-          );
-          return;
-        }
-
-        const deliveries = message.get('delivered') || 0;
-        const deliveredTo = message.get('delivered_to') || [];
-        const expirationStartTimestamp = message.get('expirationStartTimestamp');
-        message.set({
-          delivered_to: _.union(deliveredTo, [receipt.get('source')]),
-          delivered: deliveries + 1,
-          expirationStartTimestamp: expirationStartTimestamp || Date.now(),
-          sent: true,
-        });
-
-        if (message.isExpiring() && !expirationStartTimestamp) {
-          // This will save the message for us while starting the timer
-          await message.setToExpire();
-        } else {
-          await message.commit();
-        }
-
-        // notify frontend listeners
-        const conversation = window.getConversationController().get(message.get('conversationId'));
-        if (conversation) {
-          conversation.updateLastMessage();
-        }
-
-        this.remove(receipt);
-      } catch (error) {
-        window.log.error(
-          'DeliveryReceipts.onReceipt error:',
-          error && error.stack ? error.stack : error
-        );
-      }
-    },
-  }))();
-})();

protos/SignalService.proto

@@ -176,7 +176,6 @@ message ConfigurationMessage {
 message ReceiptMessage {
 
   enum Type {
-    DELIVERY = 0;
     READ     = 1;
   }
 

test/fixtures.js

@@ -229,7 +229,6 @@ Whisper.Fixtures = () => {
     type: 'outgoing',
     body: 'See you all there!',
     recipients: [MICHEL_ID, FRED_ID, NESTOR_ID],
-    delivered_to: [MICHEL_ID, FRED_ID],
     sent_to: [NESTOR_ID],
     conversationId: group.id,
   });

ts/components/conversation/ExpireTimer.tsx

@@ -5,12 +5,13 @@ import { useInterval } from '../../hooks/useInterval';
 import styled, { DefaultTheme } from 'styled-components';
 import { OpacityMetadataComponent } from './message/MessageMetadata';
 import { SessionIcon, SessionIconSize } from '../session/icon';
+import { MessageModelType } from '../../models/messageType';
 
 type Props = {
   withImageNoCaption: boolean;
   expirationLength: number;
   expirationTimestamp: number;
-  direction: 'incoming' | 'outgoing';
+  direction: MessageModelType;
   theme: DefaultTheme;
 };
 

ts/components/conversation/message/MessageMetadata.tsx

@@ -1,13 +1,11 @@
 import React from 'react';
-import classNames from 'classnames';
-
 import { MessageSendingErrorText, MetadataSpacer } from './MetadataUtilComponent';
 import { OutgoingMessageStatus } from './OutgoingMessageStatus';
-import { Spinner } from '../../basic/Spinner';
 import { MetadataBadges } from './MetadataBadge';
 import { Timestamp } from '../Timestamp';
 import { ExpireTimer } from '../ExpireTimer';
 import styled, { DefaultTheme } from 'styled-components';
+import { MessageDeliveryStatus, MessageModelType } from '../../../models/messageType';
 
 type Props = {
   disableMenu?: boolean;
@@ -16,10 +14,10 @@ type Props = {
   text?: string;
   id: string;
   collapseMetadata?: boolean;
-  direction: 'incoming' | 'outgoing';
+  direction: MessageModelType;
   timestamp: number;
   serverTimestamp?: number;
-  status?: 'sending' | 'sent' | 'delivered' | 'read' | 'error';
+  status?: MessageDeliveryStatus;
   expirationLength?: number;
   expirationTimestamp?: number;
   isPublic?: boolean;

ts/components/conversation/message/OutgoingMessageStatus.tsx

@@ -1,5 +1,6 @@
 import React from 'react';
 import styled, { DefaultTheme } from 'styled-components';
+import { MessageDeliveryStatus } from '../../../models/messageType';
 import { SessionIcon, SessionIconSize, SessionIconType } from '../../session/icon';
 import { OpacityMetadataComponent } from './MessageMetadata';
 
@@ -36,19 +37,6 @@ const MessageStatusSent = (props: { theme: DefaultTheme; iconColor: string }) =>
   );
 };
 
-const MessageStatusDelivered = (props: { theme: DefaultTheme; iconColor: string }) => {
-  return (
-    <MessageStatusSendingContainer>
-      <SessionIcon
-        iconColor={props.iconColor}
-        theme={props.theme}
-        iconType={SessionIconType.DoubleCheckCircle}
-        iconSize={SessionIconSize.Tiny}
-      />
-    </MessageStatusSendingContainer>
-  );
-};
-
 const MessageStatusRead = (props: { theme: DefaultTheme; iconColor: string }) => {
   return (
     <MessageStatusSendingContainer>
@@ -76,7 +64,7 @@ const MessageStatusError = (props: { theme: DefaultTheme }) => {
 };
 
 export const OutgoingMessageStatus = (props: {
-  status?: 'sending' | 'sent' | 'delivered' | 'read' | 'error';
+  status?: MessageDeliveryStatus;
   theme: DefaultTheme;
   iconColor: string;
   isInMessageView?: boolean;
@@ -86,8 +74,6 @@ export const OutgoingMessageStatus = (props: {
       return <MessageStatusSending {...props} />;
     case 'sent':
       return <MessageStatusSent {...props} />;
-    case 'delivered':
-      return <MessageStatusDelivered {...props} />;
     case 'read':
       return <MessageStatusRead {...props} />;
     case 'error':

ts/models/message.ts

@@ -463,11 +463,6 @@ export class MessageModel extends Backbone.Model<MessageAttributes> {
     if (window.storage.get('read-receipt-setting') && readBy.length > 0) {
       return 'read';
     }
-    const delivered = this.get('delivered');
-    const deliveredTo = this.get('delivered_to') || [];
-    if (delivered || deliveredTo.length > 0) {
-      return 'delivered';
-    }
     const sent = this.get('sent');
     const sentTo = this.get('sent_to') || [];
     if (sent || sentTo.length > 0) {
@@ -978,10 +973,6 @@ export class MessageModel extends Backbone.Model<MessageAttributes> {
     if (readBy.indexOf(pubkey) >= 0) {
       return 'read';
     }
-    const deliveredTo = this.get('delivered_to') || [];
-    if (deliveredTo.indexOf(pubkey) >= 0) {
-      return 'delivered';
-    }
     const sentTo = this.get('sent_to') || [];
     if (sentTo.indexOf(pubkey) >= 0) {
       return 'sent';

ts/models/messageType.ts

@@ -7,7 +7,7 @@ import { Contact } from '../types/Contact';
 import { ConversationTypeEnum } from './conversation';
 
 export type MessageModelType = 'incoming' | 'outgoing';
-export type MessageDeliveryStatus = 'sending' | 'sent' | 'delivered' | 'read' | 'error';
+export type MessageDeliveryStatus = 'sending' | 'sent' | 'read' | 'error';
 
 export interface MessageAttributes {
   // the id of the message
@@ -23,11 +23,9 @@ export interface MessageAttributes {
   body?: string;
   expirationStartTimestamp: number;
   read_by: Array<string>;
-  delivered_to: Array<string>;
   decrypted_at: number;
   expires_at?: number;
   recipients: Array<string>;
-  delivered?: number;
   type: MessageModelType;
   group_update?: any;
   groupInvitation?: any;
@@ -127,11 +125,9 @@ export interface MessageAttributesOptionals {
   body?: string;
   expirationStartTimestamp?: number;
   read_by?: Array<string>;
-  delivered_to?: Array<string>;
   decrypted_at?: number;
   expires_at?: number;
   recipients?: Array<string>;
-  delivered?: number;
   type: MessageModelType;
   group_update?: any;
   groupInvitation?: any;
@@ -178,13 +174,22 @@ export interface MessageAttributesOptionals {
 export const fillMessageAttributesWithDefaults = (
   optAttributes: MessageAttributesOptionals
 ): MessageAttributes => {
-  //FIXME to do put the default
+  const defaulted = _.defaults(optAttributes, {
-  return _.defaults(optAttributes, {
     expireTimer: 0, // disabled
     id: uuidv4(),
     schemaVersion: window.Signal.Types.Message.CURRENT_SCHEMA_VERSION,
     unread: 0, // if nothing is set, this message is considered read
   });
+  // this is just to cleanup a bit the db. delivered and delivered_to were removed, so everytime we load a message
+  // we make sure to clean those fields in the json.
+  // the next commit() will write that to the disk
+  if (defaulted.delivered) {
+    delete defaulted.delivered;
+  }
+  if (defaulted.delivered_to) {
+    delete defaulted.delivered_to;
+  }
+  return defaulted;
 };
 
 export interface MessageRegularProps {
@@ -195,10 +200,10 @@ export interface MessageRegularProps {
   text?: string;
   id: string;
   collapseMetadata?: boolean;
-  direction: 'incoming' | 'outgoing';
+  direction: MessageModelType;
   timestamp: number;
   serverTimestamp?: number;
-  status?: 'sending' | 'sent' | 'delivered' | 'read' | 'error';
+  status?: MessageDeliveryStatus;
   // What if changed this over to a single contact like quote, and put the events on it?
   contact?: Contact & {
     onSendMessage?: () => void;

ts/receiver/contentMessage.ts

@@ -383,20 +383,6 @@ function onReadReceipt(readAt: any, timestamp: any, reader: any) {
   return Whisper.ReadReceipts.onReceipt(receipt);
 }
 
-export function onDeliveryReceipt(source: any, timestamp: any) {
-  const { Whisper } = window;
-
-  window?.log?.info('delivery receipt from', `${source}.${1}`, timestamp);
-
-  const receipt = Whisper.DeliveryReceipts.add({
-    timestamp,
-    source,
-  });
-
-  // Calling this directly so we can wait for completion
-  return Whisper.DeliveryReceipts.onReceipt(receipt);
-}
-
 async function handleReceiptMessage(
   envelope: EnvelopePlus,
   receiptMessage: SignalService.IReceiptMessage
@@ -406,12 +392,7 @@ async function handleReceiptMessage(
   const { type, timestamp } = receipt;
 
   const results = [];
-  if (type === SignalService.ReceiptMessage.Type.DELIVERY) {
+  if (type === SignalService.ReceiptMessage.Type.READ) {
-    for (const ts of timestamp) {
-      const promise = onDeliveryReceipt(envelope.source, Lodash.toNumber(ts));
-      results.push(promise);
-    }
-  } else if (type === SignalService.ReceiptMessage.Type.READ) {
     for (const ts of timestamp) {
       const promise = onReadReceipt(
         Lodash.toNumber(envelope.timestamp),

ts/receiver/dataMessage.ts

@@ -19,7 +19,6 @@ import {
   getMessageBySenderAndServerTimestamp,
 } from '../../ts/data/data';
 import { ConversationModel, ConversationTypeEnum } from '../models/conversation';
-import { DeliveryReceiptMessage } from '../session/messages/outgoing/controlMessage/receipt/DeliveryReceiptMessage';
 import { allowOnlyOneAtATime } from '../session/utils/Promise';
 
 export async function updateProfileOneAtATime(
@@ -553,15 +552,6 @@ export function createMessage(data: MessageCreationData, isIncoming: boolean): M
   }
 }
 
-function sendDeliveryReceipt(source: string, timestamp: any) {
-  const receiptMessage = new DeliveryReceiptMessage({
-    timestamp: Date.now(),
-    timestamps: [timestamp],
-  });
-  const device = new PubKey(source);
-  void getMessageQueue().sendToPubKey(device, receiptMessage);
-}
-
 export interface MessageEvent {
   data: any;
   type: string;
@@ -605,12 +595,6 @@ export async function handleMessageEvent(event: MessageEvent): Promise<void> {
 
   const isOurDevice = UserUtils.isUsFromCache(source);
 
-  const shouldSendReceipt = isIncoming && !isGroupMessage && !isOurDevice;
-
-  if (shouldSendReceipt) {
-    sendDeliveryReceipt(source, data.timestamp);
-  }
-
   // Conversation Id is:
   //  - primarySource if it is an incoming DM message,
   //  - destination if it is an outgoing message,

ts/receiver/queuedJob.ts

@@ -258,37 +258,17 @@ function handleSyncedReceipts(message: MessageModel, conversation: ConversationM
     });
   }
 
-  const deliveryReceipts = window.Whisper.DeliveryReceipts.forMessage(conversation, message);
+  // A sync'd message to ourself is automatically considered read
-
-  if (deliveryReceipts.length) {
-    handleSyncDeliveryReceipts(message, deliveryReceipts);
-  }
-
-  // A sync'd message to ourself is automatically considered read and delivered
   const recipients = conversation.getRecipients();
   if (conversation.isMe()) {
     message.set({
       read_by: recipients,
-      delivered_to: recipients,
     });
   }
 
   message.set({ recipients });
 }
 
-function handleSyncDeliveryReceipts(message: MessageModel, receipts: any) {
-  const sources = receipts.map((receipt: any) => receipt.get('source'));
-
-  const deliveredTo = _.union(message.get('delivered_to') || [], sources);
-
-  const deliveredCount = deliveredTo.length;
-
-  message.set({
-    delivered: deliveredCount,
-    delivered_to: deliveredTo,
-  });
-}
-
 async function handleRegularMessage(
   conversation: ConversationModel,
   message: MessageModel,

ts/session/messages/outgoing/controlMessage/receipt/DeliveryReceiptMessage.ts

@@ -1,8 +0,0 @@
-import { SignalService } from '../../../../../protobuf';
-import { ReceiptMessage } from './ReceiptMessage';
-
-export class DeliveryReceiptMessage extends ReceiptMessage {
-  public getReceiptType(): SignalService.ReceiptMessage.Type {
-    return SignalService.ReceiptMessage.Type.DELIVERY;
-  }
-}

ts/state/ducks/conversations.ts

@@ -6,6 +6,7 @@ import { ConversationController } from '../../session/conversations';
 import { MessageModel } from '../../models/message';
 import { getMessagesByConversation } from '../../data/data';
 import { ConversationTypeEnum } from '../../models/conversation';
+import { MessageDeliveryStatus } from '../../models/messageType';
 
 // State
 
@@ -50,7 +51,7 @@ export type MessageTypeInConvo = {
   getPropsForMessageDetail(): Promise<any>;
 };
 
-export type LastMessageStatusType = 'error' | 'sending' | 'sent' | 'delivered' | 'read' | null;
+export type LastMessageStatusType = MessageDeliveryStatus | null;
 
 export type LastMessageType = {
   status: LastMessageStatusType;

ts/test/session/unit/messages/ReceiptMessage_test.ts

@@ -4,19 +4,16 @@ import { beforeEach } from 'mocha';
 import { SignalService } from '../../../../protobuf';
 import { toNumber } from 'lodash';
 import { Constants } from '../../../../session';
-import { DeliveryReceiptMessage } from '../../../../session/messages/outgoing/controlMessage/receipt/DeliveryReceiptMessage';
 import { ReadReceiptMessage } from '../../../../session/messages/outgoing/controlMessage/receipt/ReadReceiptMessage';
 
 describe('ReceiptMessage', () => {
   let readMessage: ReadReceiptMessage;
-  let deliveryMessage: ReadReceiptMessage;
   let timestamps: Array<number>;
 
   beforeEach(() => {
     timestamps = [987654321, 123456789];
     const timestamp = Date.now();
     readMessage = new ReadReceiptMessage({ timestamp, timestamps });
-    deliveryMessage = new DeliveryReceiptMessage({ timestamp, timestamps });
   });
 
   it('content of a read receipt is correct', () => {
@@ -28,18 +25,8 @@ describe('ReceiptMessage', () => {
     expect(decodedTimestamps).to.deep.equal(timestamps);
   });
 
-  it('content of a delivery receipt is correct', () => {
-    const plainText = deliveryMessage.plainTextBuffer();
-    const decoded = SignalService.Content.decode(plainText);
-
-    expect(decoded.receiptMessage).to.have.property('type', 0);
-    const decodedTimestamps = (decoded.receiptMessage?.timestamp ?? []).map(toNumber);
-    expect(decodedTimestamps).to.deep.equal(timestamps);
-  });
-
   it('correct ttl', () => {
     expect(readMessage.ttl()).to.equal(Constants.TTL_DEFAULT.TTL_MAX);
-    expect(deliveryMessage.ttl()).to.equal(Constants.TTL_DEFAULT.TTL_MAX);
   });
 
   it('has an identifier', () => {