Skip to content

Commit

Permalink
Simplify edit-related send functionality
Browse files Browse the repository at this point in the history
Co-authored-by: Scott Nonnenberg <scott@signal.org>
  • Loading branch information
automated-signal and scottnonnenberg-signal committed Jan 3, 2024
1 parent 1214949 commit a98c5ba
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 81 deletions.
45 changes: 27 additions & 18 deletions ts/jobs/helpers/sendNormalMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ import * as Bytes from '../../Bytes';
import {
getPropForTimestamp,
getTargetOfThisEditTimestamp,
setPropForTimestamp,
getChangesForPropAtTimestamp,
} from '../../util/editHelpers';
import { getMessageSentTimestamp } from '../../util/getMessageSentTimestamp';

Expand Down Expand Up @@ -115,7 +115,7 @@ export async function sendNormalMessage(
// The timestamp identifying the target of this edit; could be the original timestamp
// or the most recent edit prior to this one
const targetOfThisEditTimestamp = getTargetOfThisEditTimestamp({
message,
message: message.attributes,
targetTimestamp,
});

Expand Down Expand Up @@ -486,7 +486,7 @@ function getMessageRecipients({
const sendStateByConversationId =
getPropForTimestamp({
log,
message,
message: message.attributes,
prop: 'sendStateByConversationId',
targetTimestamp,
}) || {};
Expand Down Expand Up @@ -579,7 +579,7 @@ async function getMessageSendData({
// Figure out if we need to upload message body as an attachment.
let body = getPropForTimestamp({
log,
message,
message: message.attributes,
prop: 'body',
targetTimestamp,
});
Expand All @@ -603,7 +603,7 @@ async function getMessageSendData({
const preUploadAttachments =
getPropForTimestamp({
log,
message,
message: message.attributes,
prop: 'attachments',
targetTimestamp,
}) || [];
Expand Down Expand Up @@ -677,7 +677,7 @@ async function getMessageSendData({
expireTimer: message.get('expireTimer'),
bodyRanges: getPropForTimestamp({
log,
message,
message: message.attributes,
prop: 'bodyRanges',
targetTimestamp,
}),
Expand Down Expand Up @@ -720,7 +720,7 @@ async function uploadSingleAttachment({
const logId = `uploadSingleAttachment(${message.idForLogging()}`;
const oldAttachments = getPropForTimestamp({
log,
message,
message: message.attributes,
prop: 'attachments',
targetTimestamp,
});
Expand All @@ -742,13 +742,16 @@ async function uploadSingleAttachment({
...copyCdnFields(uploaded),
};

setPropForTimestamp({
const attributesToUpdate = getChangesForPropAtTimestamp({
log,
message,
message: message.attributes,
prop: 'attachments',
targetTimestamp,
value: newAttachments,
});
if (attributesToUpdate) {
message.set(attributesToUpdate);
}

return uploaded;
}
Expand All @@ -772,7 +775,7 @@ async function uploadMessageQuote({
// sends are failing, let's not add the complication of a cache.
const startingQuote = getPropForTimestamp({
log,
message,
message: message.attributes,
prop: 'quote',
targetTimestamp,
});
Expand Down Expand Up @@ -808,7 +811,7 @@ async function uploadMessageQuote({
const logId = `uploadMessageQuote(${message.idForLogging()}`;
const oldQuote = getPropForTimestamp({
log,
message,
message: message.attributes,
prop: 'quote',
targetTimestamp,
});
Expand Down Expand Up @@ -842,13 +845,16 @@ async function uploadMessageQuote({
};
}),
};
setPropForTimestamp({
const attributesToUpdate = getChangesForPropAtTimestamp({
log,
message,
message: message.attributes,
prop: 'quote',
targetTimestamp,
value: newQuote,
});
if (attributesToUpdate) {
message.set(attributesToUpdate);
}

return {
isGiftBadge: loadedQuote.isGiftBadge,
Expand Down Expand Up @@ -879,7 +885,7 @@ async function uploadMessagePreviews({
// attachments.
const startingPreview = getPropForTimestamp({
log,
message,
message: message.attributes,
prop: 'preview',
targetTimestamp,
});
Expand Down Expand Up @@ -919,7 +925,7 @@ async function uploadMessagePreviews({
const logId = `uploadMessagePreviews(${message.idForLogging()}`;
const oldPreview = getPropForTimestamp({
log,
message,
message: message.attributes,
prop: 'preview',
targetTimestamp,
});
Expand All @@ -945,13 +951,16 @@ async function uploadMessagePreviews({
};
});

setPropForTimestamp({
const attributesToUpdate = getChangesForPropAtTimestamp({
log,
message,
message: message.attributes,
prop: 'preview',
targetTimestamp,
value: newPreview,
});
if (attributesToUpdate) {
message.set(attributesToUpdate);
}

return uploadedPreviews;
}
Expand Down Expand Up @@ -1119,7 +1128,7 @@ function didSendToEveryone({
const sendStateByConversationId =
getPropForTimestamp({
log,
message,
message: message.attributes,
prop: 'sendStateByConversationId',
targetTimestamp,
}) || {};
Expand Down
62 changes: 43 additions & 19 deletions ts/models/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,10 @@ import { getSenderIdentifier } from '../util/getSenderIdentifier';
import { getNotificationDataForMessage } from '../util/getNotificationDataForMessage';
import { getNotificationTextForMessage } from '../util/getNotificationTextForMessage';
import { getMessageAuthorText } from '../util/getMessageAuthorText';
import { getPropForTimestamp, setPropForTimestamp } from '../util/editHelpers';
import {
getPropForTimestamp,
getChangesForPropAtTimestamp,
} from '../util/editHelpers';
import { getMessageSentTimestamp } from '../util/getMessageSentTimestamp';

/* eslint-disable more/no-then */
Expand Down Expand Up @@ -838,7 +841,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
const targetTimestamp = editMessageTimestamp || this.get('timestamp');
const sendStateByConversationId = getPropForTimestamp({
log,
message: this,
message: this.attributes,
prop: 'sendStateByConversationId',
targetTimestamp,
});
Expand All @@ -852,13 +855,16 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
})
);

setPropForTimestamp({
const attributesToUpdate = getChangesForPropAtTimestamp({
log,
message: this,
message: this.attributes,
prop: 'sendStateByConversationId',
targetTimestamp,
value: newSendStateByConversationId,
});
if (attributesToUpdate) {
this.set(attributesToUpdate);
}

// We aren't trying to send this message anymore, so we'll delete these caches
delete this.cachedOutgoingContactData;
Expand Down Expand Up @@ -956,7 +962,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
const sendStateByConversationId = {
...(getPropForTimestamp({
log,
message: this,
message: this.attributes,
prop: 'sendStateByConversationId',
targetTimestamp,
}) || {}),
Expand Down Expand Up @@ -1101,22 +1107,22 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
// We may overwrite this in the `saveErrors` call below.
attributesToUpdate.errors = [];

this.set(attributesToUpdate);
const additionalProps = getChangesForPropAtTimestamp({
log,
message: this.attributes,
prop: 'sendStateByConversationId',
targetTimestamp,
value: sendStateByConversationId,
});

this.set({ ...attributesToUpdate, ...additionalProps });
if (saveErrors) {
saveErrors(errorsToSave);
} else {
// We skip save because we'll save in the next step.
void this.saveErrors(errorsToSave, { skipSave: true });
}

setPropForTimestamp({
log,
message: this,
prop: 'sendStateByConversationId',
targetTimestamp,
value: sendStateByConversationId,
});

if (!this.doNotSave) {
await window.Signal.Data.saveMessage(this.attributes, {
ourAci: window.textsecure.storage.user.getCheckedAci(),
Expand Down Expand Up @@ -1237,7 +1243,12 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
const conv = this.getConversation()!;

const sendEntries = Object.entries(
this.get('sendStateByConversationId') || {}
getPropForTimestamp({
log,
message: this.attributes,
prop: 'sendStateByConversationId',
targetTimestamp,
}) || {}
);
const sentEntries = filter(sendEntries, ([_conversationId, { status }]) =>
isSent(status)
Expand Down Expand Up @@ -1292,7 +1303,12 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
).then(async result => {
let newSendStateByConversationId: undefined | SendStateByConversationId;
const sendStateByConversationId =
this.get('sendStateByConversationId') || {};
getPropForTimestamp({
log,
message: this.attributes,
prop: 'sendStateByConversationId',
targetTimestamp,
}) || {};
const ourOldSendState = getOwn(
sendStateByConversationId,
ourConversation.id
Expand All @@ -1310,12 +1326,20 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
}
}

const attributesForUpdate = newSendStateByConversationId
? getChangesForPropAtTimestamp({
log,
message: this.attributes,
prop: 'sendStateByConversationId',
value: newSendStateByConversationId,
targetTimestamp,
})
: null;

this.set({
synced: true,
dataMessage: null,
...(newSendStateByConversationId
? { sendStateByConversationId: newSendStateByConversationId }
: {}),
...attributesForUpdate,
});

// Return early, skip the save
Expand Down

0 comments on commit a98c5ba

Please sign in to comment.