Correctly handle server errors in multi-deletion

js/models/conversations.js

@@ -2487,30 +2487,32 @@
         return false;
       }
 
-      let success;
-      const shouldBeDeleted = [];
-      if (messages.length > 1) {
-        success = await channelAPI.deleteMessages(
-          messages.map(m => m.getServerId())
-        );
-      } else {
-        success = await channelAPI.deleteMessages([messages[0].getServerId()]);
-      }
+      const invalidMessages = messages.filter(m => !m.getServerId());
+      const pendingMessages = messages.filter(m => m.getServerId());
 
-      messages.forEach(m => {
+      let deletedServerIds = [];
+      let ignoredServerIds = [];
 
-        // If the message has errors it is likely not saved
-        // on the server, so we delete it locally unconditionally
-        const shouldDeleteLocally = success || m.hasErrors() || !m.getServerId();
-
-        if (shouldDeleteLocally) {
-          this.removeMessage(m.id);
+      if (pendingMessages.length > 0) {
+        const result = await channelAPI.deleteMessages(
+          pendingMessages.map(m => m.getServerId())
+        );
+        deletedServerIds = result.deletedIds;
+        ignoredServerIds = result.ignoredIds;
       }
 
-      });
+      const toDeleteLocallyServerIds = _.union(
+        deletedServerIds,
+        ignoredServerIds
+      );
+      let toDeleteLocally = messages.filter(m =>
+        toDeleteLocallyServerIds.includes(m.getServerId())
+      );
+      toDeleteLocally = _.union(toDeleteLocally, invalidMessages);
+
+      toDeleteLocally.forEach(m => this.removeMessage(m.id));
 
-      /// TODO: not sure what to return here
-      return shouldDeleteLocally;
+      return toDeleteLocally;
     },
 
     removeMessage(messageId) {

js/modules/loki_app_dot_net_api.js

@@ -525,18 +525,42 @@ class LokiPublicChannelAPI {
       this.modStatus ? `loki/v1/moderation/messages` : `loki/v1/messages`,
       { method: 'DELETE', params: { ids: serverIds } }
     );
-    if (!res.err && res.response) {
+    if (!res.err) {
+      const deletedIds = res.response.data
+        .filter(d => d.is_deleted)
+        .map(d => d.id);
+
+      if (deletedIds.length > 0) {
         log.info(`deleted ${serverIds} on ${this.baseChannelUrl}`);
-      return true;
       }
-    // fire an alert
-    log.warn(`failed to delete ${serverIds} on ${this.baseChannelUrl}`);
+
+      const failedIds = res.response.data
+        .filter(d => !d.is_deleted)
+        .map(d => d.id);
+
+      if (failedIds.length > 0) {
+        log.warn(`failed to delete ${failedIds} on ${this.baseChannelUrl}`);
+      }
+
+      // Note: if there is no entry for message, we assume it wasn't found
+      // on the server, so it is not treated as explicitly failed
+      const ignoredIds = _.difference(
+        serverIds,
+        _.union(failedIds, deletedIds)
+      );
+
+      if (ignoredIds.length > 0) {
+        log.warn(`No response for ${ignoredIds} on ${this.baseChannelUrl}`);
+      }
+
+      return { deletedIds, ignoredIds };
+    }
     if (canThrow) {
       throw new textsecure.PublicChatError(
         'Failed to delete public chat message'
       );
     }
-    return false;
+    return { deletedIds: [], ignoredIds: [] };
   }
 
   // used for sending messages

js/views/conversation_view.js

@@ -1385,18 +1385,21 @@
       })();
 
       const doDelete = async () => {
+        let toDeleteLocally;
+
         if (this.model.isPublic()) {
-          const success = await this.model.deletePublicMessages(messages);
-          if (!success) {
+          toDeleteLocally = await this.model.deletePublicMessages(messages);
+          if (toDeleteLocally.length === 0) {
             // Message failed to delete from server, show error?
             return;
           }
         } else {
           messages.forEach(m => this.model.messageCollection.remove(m.id));
+          toDeleteLocally = messages;
         }
 
         await Promise.all(
-          messages.map(async m => {
+          toDeleteLocally.map(async m => {
             await window.Signal.Data.removeMessage(m.id, {
               Message: Whisper.Message,
             });

ts/components/conversation/Message.tsx

@@ -1076,9 +1076,7 @@ export class Message extends React.PureComponent<Props, State> {
             )}
           >
             {this.renderError(isIncoming)}
-            {isRss
-              ? null
-              : this.renderMenu(!isIncoming, triggerId)}
+            {isRss ? null : this.renderMenu(!isIncoming, triggerId)}
             <div
               className={classNames(
                 'module-message__container',
@@ -1101,7 +1099,7 @@ export class Message extends React.PureComponent<Props, State> {
               {this.renderSendMessageButton()}
             </div>
             {this.renderError(!isIncoming)}
-            {(isRss || multiSelectMode)
+            {isRss || multiSelectMode
               ? null
               : this.renderMenu(isIncoming, triggerId)}
             {multiSelectMode ? null : this.renderContextMenu(triggerId)}

tslint.json

@@ -145,7 +145,10 @@
         "method": "render",
         "comment": "Usage has been approved by Ryan Tharp on 2019-07-22"
       }
-    ]
+    ],
+    // Reasonable functions can exceed the default of 100 lines
+    // due to auto-formatting
+    "max-func-body-length": [true, 150]
   },
   "rulesDirectory": ["node_modules/tslint-microsoft-contrib"]
 }