From 11ea5954572435ef044740cd8fe7af4075afbb20 Mon Sep 17 00:00:00 2001 From: Jamie Kyle <113370520+jamiebuilds-signal@users.noreply.github.com> Date: Fri, 15 Mar 2024 12:11:48 -0700 Subject: [PATCH] Remove accepted message without explicit user action --- ts/models/conversations.ts | 14 +++++---- ts/test-mock/benchmarks/group_send_bench.ts | 2 +- ts/test-mock/pnp/merge_test.ts | 9 ++---- ts/test-mock/pnp/phone_discovery_test.ts | 5 +--- ts/test-mock/pnp/pni_change_test.ts | 32 +++++++++------------ ts/test-mock/pnp/pni_signature_test.ts | 5 +--- 6 files changed, 28 insertions(+), 39 deletions(-) diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index 3265834887..fa647ffd2c 100644 --- a/ts/models/conversations.ts +++ b/ts/models/conversations.ts @@ -2163,11 +2163,15 @@ export class ConversationModel extends window.Backbone if (didResponseChange) { if (response === messageRequestEnum.ACCEPT) { - drop( - this.addMessageRequestResponseEventMessage( - MessageRequestResponseEvent.ACCEPT - ) - ); + // Only add a message when the user took an explicit action to accept + // the message request on one of their devices + if (!viaStorageServiceSync) { + drop( + this.addMessageRequestResponseEventMessage( + MessageRequestResponseEvent.ACCEPT + ) + ); + } } if ( response === messageRequestEnum.BLOCK || diff --git a/ts/test-mock/benchmarks/group_send_bench.ts b/ts/test-mock/benchmarks/group_send_bench.ts index f69f56a0b0..c585ee5012 100644 --- a/ts/test-mock/benchmarks/group_send_bench.ts +++ b/ts/test-mock/benchmarks/group_send_bench.ts @@ -114,7 +114,7 @@ Bootstrap.benchmark(async (bootstrap: Bootstrap): Promise => { const item = leftPane .locator( '.module-conversation-list__item--contact-or-conversation' + - '>> text="You accepted the message request"' + `>> text="${LAST_MESSAGE}"` ) .first(); await item.click({ timeout: 2 * MINUTE }); diff --git a/ts/test-mock/pnp/merge_test.ts b/ts/test-mock/pnp/merge_test.ts index 246373c7e8..a173f84b5b 100644 --- a/ts/test-mock/pnp/merge_test.ts +++ b/ts/test-mock/pnp/merge_test.ts @@ -149,7 +149,7 @@ describe('pnp/merge', function (this: Mocha.Suite) { assert.strictEqual(await messages.count(), 0, 'message count'); await expectSystemMessages(window, [ - 'You accepted the message request', + // none ]); } @@ -210,21 +210,16 @@ describe('pnp/merge', function (this: Mocha.Suite) { if (withPNIMessage) { if (pniSignatureVerified) { await expectSystemMessages(window, [ - 'You accepted the message request', - 'You accepted the message request', /Your message history with ACI Contact and their number .* has been merged\./, ]); } else { await expectSystemMessages(window, [ - 'You accepted the message request', - 'You accepted the message request', /Your message history with ACI Contact and their number .* has been merged\./, ]); } } else { await expectSystemMessages(window, [ - 'You accepted the message request', - 'You accepted the message request', + // none ]); } } diff --git a/ts/test-mock/pnp/phone_discovery_test.ts b/ts/test-mock/pnp/phone_discovery_test.ts index e4e65b0082..b469e51058 100644 --- a/ts/test-mock/pnp/phone_discovery_test.ts +++ b/ts/test-mock/pnp/phone_discovery_test.ts @@ -144,10 +144,7 @@ describe('pnp/phone discovery', function (this: Mocha.Suite) { const messages = window.locator('.module-message__text'); assert.strictEqual(await messages.count(), 1, 'message count'); - await expectSystemMessages(window, [ - 'You accepted the message request', - /.* belongs to ACI Contact/, - ]); + await expectSystemMessages(window, [/.* belongs to ACI Contact/]); } }); }); diff --git a/ts/test-mock/pnp/pni_change_test.ts b/ts/test-mock/pnp/pni_change_test.ts index d845c7f471..e034178ed7 100644 --- a/ts/test-mock/pnp/pni_change_test.ts +++ b/ts/test-mock/pnp/pni_change_test.ts @@ -96,9 +96,9 @@ describe('pnp/PNI Change', function (this: Mocha.Suite) { // No messages const messages = window.locator('.module-message__text'); assert.strictEqual(await messages.count(), 0, 'message count'); - - // No notifications - await expectSystemMessages(window, ['You accepted the message request']); + await expectSystemMessages(window, [ + // none + ]); } debug('Send message to contactA'); @@ -165,10 +165,7 @@ describe('pnp/PNI Change', function (this: Mocha.Suite) { assert.strictEqual(await messages.count(), 1, 'message count'); // Only a PhoneNumberDiscovery notification - await expectSystemMessages(window, [ - 'You accepted the message request', - /.* belongs to ContactA/, - ]); + await expectSystemMessages(window, [/.* belongs to ContactA/]); } }); @@ -198,7 +195,9 @@ describe('pnp/PNI Change', function (this: Mocha.Suite) { const messages = window.locator('.module-message__text'); assert.strictEqual(await messages.count(), 0, 'message count'); - await expectSystemMessages(window, ['You accepted the message request']); + await expectSystemMessages(window, [ + // 'You accepted the message request' + ]); } debug('Send message to contactA'); @@ -266,7 +265,6 @@ describe('pnp/PNI Change', function (this: Mocha.Suite) { // Two notifications - the safety number change and PhoneNumberDiscovery await expectSystemMessages(window, [ - 'You accepted the message request', /.* belongs to ContactA/, /Safety Number has changed/, ]); @@ -299,7 +297,9 @@ describe('pnp/PNI Change', function (this: Mocha.Suite) { const messages = window.locator('.module-message__text'); assert.strictEqual(await messages.count(), 0, 'message count'); - await expectSystemMessages(window, ['You accepted the message request']); + await expectSystemMessages(window, [ + // none + ]); } debug('Send message to contactA'); @@ -397,7 +397,6 @@ describe('pnp/PNI Change', function (this: Mocha.Suite) { // Three notifications - accepted, the safety number change and PhoneNumberDiscovery await expectSystemMessages(window, [ - 'You accepted the message request', /.* belongs to ContactA/, /Safety Number has changed/, ]); @@ -429,9 +428,9 @@ describe('pnp/PNI Change', function (this: Mocha.Suite) { // No messages const messages = window.locator('.module-message__text'); assert.strictEqual(await messages.count(), 0, 'message count'); - - // No notifications - await expectSystemMessages(window, ['You accepted the message request']); + await expectSystemMessages(window, [ + // none + ]); } debug('Send message to contactA'); @@ -551,10 +550,7 @@ describe('pnp/PNI Change', function (this: Mocha.Suite) { assert.strictEqual(await messages.count(), 2, 'message count'); // Only a PhoneNumberDiscovery notification - await expectSystemMessages(window, [ - 'You accepted the message request', - /.* belongs to ContactA/, - ]); + await expectSystemMessages(window, [/.* belongs to ContactA/]); } }); }); diff --git a/ts/test-mock/pnp/pni_signature_test.ts b/ts/test-mock/pnp/pni_signature_test.ts index 2ce00e020b..1347861a5a 100644 --- a/ts/test-mock/pnp/pni_signature_test.ts +++ b/ts/test-mock/pnp/pni_signature_test.ts @@ -423,10 +423,7 @@ describe('pnp/PNI Signature', function (this: Mocha.Suite) { assert.strictEqual(await messages.count(), 3, 'messages'); // Title transition notification - await expectSystemMessages(window, [ - 'You accepted the message request', - /You started this chat with/, - ]); + await expectSystemMessages(window, [/You started this chat with/]); assert.isEmpty(await phone.getOrphanedStorageKeys()); }