Remove Conversation Message Preview After Message Has Expired

- [x] Skip timestamp update for `verified-change` and expiring messages to
      prevent them to bump to the top of the conversation list.
- [x] Skip message preview (`lastMessage`) update for `verified-change` messages
      since they don’t contain any message body.
- [x] Reset last message if conversation is empty.

Fixes https://github.com/signalapp/Signal-Desktop/issues/980.

.editorconfig

@@ -10,5 +10,5 @@ indent_style = space
 insert_final_newline = true
 trim_trailing_whitespace = true
 
-[{js/modules/**/*.js, test/app/**/*.js, test/modules/**/*.js}]
+[{js/modules/**/*.js, ts/**/*.ts, test/app/**/*.js, test/modules/**/*.js}]
 indent_size = 2

js/models/conversations.js

@@ -669,31 +669,28 @@
         ));
       });
     },
-    /* jshint ignore:end */
-    /* eslint-disable */
 
-    updateLastMessage: function() {
+    async updateLastMessage() {
-        var collection = new Whisper.MessageCollection();
+      const collection = new Whisper.MessageCollection();
-        return collection.fetchConversation(this.id, 1).then(function() {
+      await collection.fetchConversation(this.id, 1);
-            var lastMessage = collection.at(0);
+      const lastMessage = collection.at(0);
-            if (lastMessage) {
+
-                var type = lastMessage.get('type');
+      const lastMessageUpdate = Signal.Types.Conversation.createLastMessageUpdate({
-                var shouldSkipUpdate = type === 'verified-change' || lastMessage.get('expirationTimerUpdate');
+        currentLastMessageText: this.get('lastMessage') || null,
-                if (shouldSkipUpdate) {
+        currentTimestamp: this.get('timestamp') || null,
-                    return;
+        lastMessage: lastMessage ? lastMessage.toJSON() : null,
-                }
+        lastMessageNotificationText: lastMessage
-                this.set({
+          ? lastMessage.getNotificationText() : null,
-                   lastMessage : lastMessage.getNotificationText(),
+      });
-                   timestamp   : lastMessage.get('sent_at')
+
-                });
+      this.set(lastMessageUpdate);
-            } else {
+
-                this.set({ lastMessage: '', timestamp: null });
+      if (this.hasChanged('lastMessage') || this.hasChanged('timestamp')) {
-            }
+        this.save();
-            if (this.hasChanged('lastMessage') || this.hasChanged('timestamp')) {
+      }
-                this.save();
-            }
-        }.bind(this));
     },
+    /* jshint ignore:end */
+    /* eslint-disable */
 
     updateExpirationTimer: function(expireTimer, source, received_at, options) {
         options = options || {};

package.json

@@ -42,7 +42,7 @@
     "jscs": "yarn grunt jscs",
     "jshint": "yarn grunt jshint",
     "lint": "yarn eslint && yarn grunt lint && yarn tslint",
-    "tslint": "tslint --config tslint.json --project . --format stylish",
+    "tslint": "tslint --config tslint.json --format stylish --project .",
     "transpile": "tsc",
     "clean-transpile": "rimraf ts/**/*.js ts/*.js",
     "open-coverage": "open coverage/lcov-report/index.html",
@@ -135,6 +135,7 @@
     "spectron": "^3.8.0",
     "ts-loader": "^4.1.0",
     "tslint": "^5.9.1",
+    "tslint-microsoft-contrib": "^5.0.3",
     "tslint-react": "^3.5.1",
     "typescript": "^2.8.1",
     "webpack": "^4.4.1"

preload.js

@@ -183,6 +183,7 @@ window.Signal.Startup = require('./js/modules/startup');
 
 window.Signal.Types = {};
 window.Signal.Types.Attachment = Attachment;
+window.Signal.Types.Conversation = require('./ts/types/Conversation');
 window.Signal.Types.Errors = require('./js/modules/types/errors');
 
 window.Signal.Types.Message = Message;

test/styleguide/legacy_bridge.js

@@ -34,6 +34,7 @@ window.Signal.Migrations.V17 = {};
 window.Signal.OS = {};
 window.Signal.Types = {};
 window.Signal.Types.Attachment = {};
+window.Signal.Types.Conversation = {};
 window.Signal.Types.Errors = {};
 window.Signal.Types.Message = {
   initializeSchemaVersion: attributes => attributes,

ts/components/utility/BackboneWrapper.tsx

@@ -22,7 +22,7 @@ interface BackboneViewConstructor {
  * while we slowly replace the internals of a given Backbone view with React.
  */
 export class BackboneWrapper extends React.Component<Props, {}> {
-  protected el: Element | null = null;
+  protected el: HTMLElement | null = null;
   protected view: BackboneView | null = null;
 
   public componentWillUnmount() {

ts/test/types/Conversation_test.ts

@@ -0,0 +1,102 @@
+import 'mocha';
+import { assert } from 'chai';
+
+import * as Conversation from '../../types/Conversation';
+import {
+  IncomingMessage,
+  OutgoingMessage,
+  VerifiedChangeMessage,
+} from '../../types/Message';
+
+describe('Conversation', () => {
+  describe('createLastMessageUpdate', () => {
+    it('should reset last message if conversation has no messages', () => {
+      const input = {
+        currentLastMessageText: null,
+        currentTimestamp: null,
+        lastMessage: null,
+        lastMessageNotificationText: null,
+      };
+      const expected = {
+        lastMessage: '',
+        timestamp: null,
+      };
+
+      const actual = Conversation.createLastMessageUpdate(input);
+      assert.deepEqual(actual, expected);
+    });
+
+    context('for regular message', () => {
+      it('should update last message text and timestamp', () => {
+        const input = {
+          currentLastMessageText: 'Existing message',
+          currentTimestamp: 555,
+          lastMessage: {
+             type: 'outgoing',
+             conversationId: 'foo',
+             sent_at: 666,
+             timestamp: 666,
+          } as OutgoingMessage,
+          lastMessageNotificationText: 'New outgoing message',
+        };
+        const expected = {
+          lastMessage: 'New outgoing message',
+          timestamp: 666,
+        };
+
+        const actual = Conversation.createLastMessageUpdate(input);
+        assert.deepEqual(actual, expected);
+      });
+    });
+    context('for verified change message', () => {
+      it('should skip update', () => {
+        const input = {
+          currentLastMessageText: 'bingo',
+          currentTimestamp: 555,
+          lastMessage: {
+             type: 'verified-change',
+             conversationId: 'foo',
+             sent_at: 666,
+             timestamp: 666,
+          } as VerifiedChangeMessage,
+          lastMessageNotificationText: 'Verified Changed',
+        };
+        const expected = {
+          lastMessage: 'bingo',
+          timestamp: 555,
+        };
+
+        const actual = Conversation.createLastMessageUpdate(input);
+        assert.deepEqual(actual, expected);
+      });
+    });
+
+    context('for expired message', () => {
+      it('should update message but not timestamp (to prevent bump to top)', () => {
+        const input = {
+          currentLastMessageText: 'I am expired',
+          currentTimestamp: 555,
+          lastMessage: {
+             type: 'incoming',
+             conversationId: 'foo',
+             sent_at: 666,
+             timestamp: 666,
+             expirationTimerUpdate: {
+               expireTimer: 111,
+               fromSync: false,
+               source: '+12223334455',
+             },
+          } as IncomingMessage,
+          lastMessageNotificationText: 'Last message before expired',
+        };
+        const expected = {
+          lastMessage: 'Last message before expired',
+          timestamp: 555,
+        };
+
+        const actual = Conversation.createLastMessageUpdate(input);
+        assert.deepEqual(actual, expected);
+      });
+    });
+  });
+});

ts/types/Attachment.ts

@@ -0,0 +1,19 @@
+import { MIMEType } from './MIME';
+
+
+export interface Attachment {
+  fileName?: string;
+  contentType?: MIMEType;
+  size?: number;
+  data: ArrayBuffer;
+
+  // // Omit unused / deprecated keys:
+  // schemaVersion?: number;
+  // id?: string;
+  // width?: number;
+  // height?: number;
+  // thumbnail?: ArrayBuffer;
+  // key?: ArrayBuffer;
+  // digest?: ArrayBuffer;
+  // flags?: number;
+}

ts/types/Conversation.ts

@@ -0,0 +1,45 @@
+import is from '@sindresorhus/is';
+import { Message } from './Message';
+
+
+interface ConversationLastMessageUpdate {
+  lastMessage: string | null;
+  timestamp: number | null;
+}
+
+export const createLastMessageUpdate = ({
+  currentLastMessageText,
+  currentTimestamp,
+  lastMessage,
+  lastMessageNotificationText,
+}: {
+  currentLastMessageText: string | null,
+  currentTimestamp: number | null,
+  lastMessage: Message | null,
+  lastMessageNotificationText: string | null,
+}): ConversationLastMessageUpdate => {
+  if (lastMessage === null) {
+    return {
+      lastMessage: '',
+      timestamp: null,
+    };
+  }
+
+  const { type } = lastMessage;
+  const isVerifiedChangeMessage = type === 'verified-change';
+  const isExpiringMessage = is.object(lastMessage.expirationTimerUpdate);
+  const shouldUpdateTimestamp = !isVerifiedChangeMessage && !isExpiringMessage;
+
+  const newTimestamp = shouldUpdateTimestamp ?
+    lastMessage.sent_at :
+    currentTimestamp;
+
+  const shouldUpdateLastMessageText = !isVerifiedChangeMessage;
+  const newLastMessageText = shouldUpdateLastMessageText ?
+    lastMessageNotificationText : currentLastMessageText;
+
+  return {
+    lastMessage: newLastMessageText,
+    timestamp: newTimestamp,
+  };
+};

ts/types/MIME.ts

@@ -0,0 +1 @@
+export type MIMEType = string & { _mimeTypeBrand: any };

ts/types/Message.ts

@@ -0,0 +1,62 @@
+import { Attachment } from './Attachment';
+
+
+export type Message
+  = IncomingMessage
+  | OutgoingMessage
+  | VerifiedChangeMessage;
+
+export type IncomingMessage = Readonly<{
+  type: 'incoming';
+  attachments: Array<Attachment>;
+  body?: string;
+  decrypted_at?: number;
+  errors?: Array<any>;
+  flags?: number;
+  id: string;
+  received_at: number;
+  source?: string;
+  sourceDevice?: number;
+} & SharedMessageProperties & Message4 & ExpirationTimerUpdate>;
+
+export type OutgoingMessage = Readonly<{
+  type: 'outgoing';
+  attachments: Array<Attachment>;
+  body?: string;
+  delivered: number;
+  delivered_to: Array<string>;
+  destination: string; // PhoneNumber
+  expirationStartTimestamp: number;
+  expires_at?: number;
+  expireTimer?: number;
+  id: string;
+  received_at: number;
+  recipients?: Array<string>; // Array<PhoneNumber>
+  sent: boolean;
+  sent_to: Array<string>; // Array<PhoneNumber>
+  synced: boolean;
+} & SharedMessageProperties & Message4 & ExpirationTimerUpdate>;
+
+export type VerifiedChangeMessage = Readonly<{
+  type: 'verified-change';
+} & SharedMessageProperties & Message4 & ExpirationTimerUpdate>;
+
+type SharedMessageProperties = Readonly<{
+  conversationId: string;
+  sent_at: number;
+  timestamp: number;
+}>;
+
+type ExpirationTimerUpdate = Readonly<{
+  expirationTimerUpdate?: Readonly<{
+    expireTimer: number;
+    fromSync: boolean;
+    source: string; // PhoneNumber
+  }>,
+}>;
+
+type Message4 = Readonly<{
+  numAttachments?: number;
+  numVisualMediaAttachments?: number;
+  numFileAttachments?: number;
+}>;

tsconfig.json

@@ -3,7 +3,10 @@
     /* Basic Options */
     "target": "es2016",                       /* Specify ECMAScript target version: 'ES3' (default), 'ES5', 'ES2015', 'ES2016', 'ES2017','ES2018' or 'ESNEXT'. */
     "module": "commonjs",                     /* Specify module code generation: 'none', 'commonjs', 'amd', 'system', 'umd', 'es2015', or 'ESNext'. */
-    // "lib": [],                             /* Specify library files to be included in the compilation. */
+    "lib": [                                  /* Specify library files to be included in the compilation. */
+        "dom",    // Required to access `window`
+        "es2017", // Required by `@sindresorhus/is`
+    ],
     // "allowJs": true,                       /* Allow javascript files to be compiled. */
     // "checkJs": true,                       /* Report errors in .js files. */
     "jsx": "react",                           /* Specify JSX code generation: 'preserve', 'react-native', or 'react'. */

tslint.json

@@ -8,6 +8,13 @@
   "rules": {
     "array-type": [true, "generic"],
     "interface-name": [true, "never-prefix"],
+
+    "mocha-avoid-only": true,
+    // Disabled until we can allow dynamically generated tests:
+    // https://github.com/Microsoft/tslint-microsoft-contrib/issues/85#issuecomment-371749352
+    "mocha-no-side-effect-code": false,
+    "mocha-unneeded-done": true,
+
     "no-consecutive-blank-lines": [true, 2],
     "object-literal-key-quotes": [true, "as-needed"],
     "object-literal-sort-keys": false,
@@ -22,5 +29,7 @@
     "quotemark": [true, "single", "jsx-double", "avoid-template", "avoid-escape"]
 
   },
-  "rulesDirectory": []
+  "rulesDirectory": [
+    "node_modules/tslint-microsoft-contrib"
+  ]
 }

yarn.lock

@@ -8866,6 +8866,12 @@ tslib@^1.8.0, tslib@^1.8.1:
   version "1.9.0"
   resolved "https://registry.yarnpkg.com/tslib/-/tslib-1.9.0.tgz#e37a86fda8cbbaf23a057f473c9f4dc64e5fc2e8"
 
+tslint-microsoft-contrib@^5.0.3:
+  version "5.0.3"
+  resolved "https://registry.yarnpkg.com/tslint-microsoft-contrib/-/tslint-microsoft-contrib-5.0.3.tgz#6fc3e238179cd72045c2b422e4d655f4183a8d5c"
+  dependencies:
+    tsutils "^2.12.1"
+
 tslint-react@^3.5.1:
   version "3.5.1"
   resolved "https://registry.yarnpkg.com/tslint-react/-/tslint-react-3.5.1.tgz#a5ca48034bf583fb63b42763bb89fa23062d5390"