Individual message per recipient #4284

rrrooommmaaa · 2022-02-13T12:57:10Z

close #4348

also this PR correctly stores threadId when saving sent messages

Tests (delete all except exactly one):

Tests added or updated

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

addresses the issue it closes (if any)
code is readable and understandable
is accompanied with tests, or tests are not needed
is free of vulnerabilities
is documented clearly and usefully, or doesn't need documentation

rrrooommmaaa · 2022-02-13T16:39:01Z

I see FES still returns { url } only, so it hasn't been updated yet?

rrrooommmaaa · 2022-02-17T12:47:18Z

ping @tomholub

rrrooommmaaa · 2022-02-23T15:39:39Z

Should we merge now or wait until FES is updated to test it again manually?

tomholub · 2022-03-07T16:53:12Z

I'll take a look

extension/chrome/elements/compose-modules/compose-send-btn-module.ts

tomholub · 2022-03-24T14:53:36Z

Sorry for lack of info. Indeed, I suppose we didn't update FES yet. I'll let you know once updated.

tomholub · 2022-04-02T13:34:32Z

We can discuss this one once rebased onto master. Thanks!

rrrooommmaaa · 2022-04-04T10:05:18Z

We can discuss this one once rebased onto master. Thanks!

The sending works, now the principal issue is how we should display the sent messages in the thread?
Previously the thread of messages was tracked by threadId. As a solitary message was sent for both pwd link and embedded encrypted message, this message was put into the thread without any problems.
Now that we send a separate encrypted message and several pwd link messages, we need to think through how we visually collapse them into one.

tomholub · 2022-04-05T12:32:34Z

Let's not worry about it in this PR, and then evaluate it in another issue/PR.

for new messages: If Gmail classifies it as many threads of one message each, then that's what it is

for replies, since we are adding thread ids, several messages will all show up in that one thread, and I suppose they'll look identical except for recipients.

In another issue, we could only actually render the first one, and then for all that directly follow and belong to the same "group" we may collapse them somehow.

Mobile apps could be more clever about this, since we have full control.

tomholub

Just some thoughts.

One thing to think about is format of the sent message. Till now, we were sending both link and encrypted data in attachment in the same message (encrypted data attached directly for any pgp recipients and the sender, and link for non-pgp). But it's not realistic to send that to 20 recipients.. so we will have to rethink that.

extension/chrome/elements/compose-modules/compose-send-btn-module.ts

extension/chrome/elements/compose-modules/formatters/encrypted-mail-msg-formatter.ts

…gateway-update

extension/chrome/elements/compose-modules/compose-send-btn-module.ts

rrrooommmaaa · 2022-06-12T11:50:03Z

@tomholub updated according to your requests, please re-review

tomholub

I like all these changes following the review - that looks good. I skipped encrypted-mail-msg-formatter.ts which I still have to look into. Plus need to do some manual testing.

tomholub

I'm filing some comments for "later" and resolving them here in this review.

The ones I leave unresolved should be done still in this PR now.

extension/chrome/elements/compose-modules/formatters/encrypted-mail-msg-formatter.ts

tomholub · 2022-06-20T09:01:32Z

extension/chrome/elements/compose-modules/formatters/encrypted-mail-msg-formatter.ts

+  private sendablePwdMsg = async (
+    newMsg: NewMsgData,
+    pubs: PubkeyResult[],
+    { msgUrl, externalId }: { msgUrl: string, externalId?: string },
+    signingPrv?: Key) => {
    // encoded as: PGP/MIME-like structure but with attachments as external files due to email size limit (encrypted for pubkeys only)
    const msgBody = this.richtext ? { 'text/plain': newMsg.plaintext, 'text/html': newMsg.plainhtml } : { 'text/plain': newMsg.plaintext };
    const pgpMimeNoAttachments = await Mime.encode(msgBody, { Subject: newMsg.subject }, []); // no attachments, attached to email separately
    const { data: pubEncryptedNoAttachments } = await this.encryptDataArmor(Buf.fromUtfStr(pgpMimeNoAttachments), undefined, pubs, signingPrv); // encrypted only for pubs
-    const attachments = this.createPgpMimeAttachments(pubEncryptedNoAttachments).
-      concat(await this.view.attachmentsModule.attachment.collectEncryptAttachments(pubs)); // encrypted only for pubs
    const emailIntroAndLinkBody = await this.formatPwdEncryptedMsgBodyLink(msgUrl);
-    return await SendableMsg.createPwdMsg(this.acctEmail, this.headers(newMsg), emailIntroAndLinkBody, attachments, { isDraft: this.isDraft, externalId });
+    return await SendableMsg.createPwdMsg(this.acctEmail, this.headers(newMsg), emailIntroAndLinkBody,
+      this.createPgpMimeAttachments(pubEncryptedNoAttachments),
+      { isDraft: this.isDraft, externalId });
  };


For a refactor in another issue:

this sendablePwdMsg method could be removed

formatSendablePwdMsgs could call const emailIntroAndLinkBody = await this.formatPwdEncryptedMsgBodyLink(msgUrl); and return await SendableMsg.createPwdMsg(this.acctEmail, this.headers(newMsg), emailIntroAndLinkBody, messageBodyAsPgpMimeAttachmentsEncryptedForSender, { isDraft: this.isDraft, externalId });

messageBodyAsPgpMimeAttachmentsEncryptedForSender could be created only once, and reused in each cycle of the loop in formatSendablePwdMsgs

to create messageBodyAsPgpMimeMetaAttachmentsEncryptedForSender you could reuse most of existing sendablePwdMsg that relates to creating this pgpmime attachments, and this new method could be called encryptMessageBodyAsPgpMimeMetaAttachments and only sender pubkeys would be passed into it to create messageBodyAsPgpMimeMetaAttachmentsEncryptedForSender

The point is that this encryption only needs to happen once per batch, not once per each pwd msg, and it only needs to use pubkeys of sender, not everybody's pubkeys.

…entMessage object

…gateway-update

…etaAttachments

… a message

tomholub

Looks good. I still need to do some manual tests before merging.

rrrooommmaaa · 2022-07-10T10:01:15Z

extension/chrome/elements/compose-modules/compose-render-module.ts

-      this.view.S.cached('reply_msg_successful').find('div.replied_cc span').text(msg.recipients.cc.join(', '));
+    this.view.S.cached('reply_msg_successful').find('div.replied_to span').text(Str.formatEmailList(recipients.to || []));
+    if (recipients.cc !== undefined && recipients.cc.length > 0) {
+      this.view.S.cached('reply_msg_successful').find('div.replied_cc span').text(Str.formatEmailList(recipients.cc));


this fixes #4497

rrrooommmaaa requested a review from tomholub February 23, 2022 15:39

rrrooommmaaa commented Mar 17, 2022

View reviewed changes

extension/chrome/elements/compose-modules/compose-send-btn-module.ts Outdated Show resolved Hide resolved

tomholub mentioned this pull request Mar 24, 2022

send one message per web portal recipient, each with different url #4348

Closed

rrrooommmaaa changed the title ~~Bind email gateway message id~~ Individual message per recipient Mar 26, 2022

rrrooommmaaa mentioned this pull request Mar 26, 2022

bind message gateway id #4353

Merged

5 tasks

wip

215a0e9

rrrooommmaaa force-pushed the issue-4269-message-gateway-update branch from 0a4d19d to 215a0e9 Compare April 4, 2022 09:45

tomholub reviewed Apr 11, 2022

View reviewed changes

extension/chrome/elements/compose-modules/compose-send-btn-module.ts Outdated Show resolved Hide resolved

extension/chrome/elements/compose-modules/formatters/encrypted-mail-msg-formatter.ts Outdated Show resolved Hide resolved

Merge remote-tracking branch 'origin/master' into issue-4269-message-…

1b0aeea

…gateway-update

rrrooommmaaa force-pushed the issue-4269-message-gateway-update branch from 184d6ae to 1b0aeea Compare April 13, 2022 12:22

rrrooommmaaa added 2 commits April 14, 2022 09:37

wip

6e1a9ec

eslint fix

6e1c417

rrrooommmaaa force-pushed the issue-4269-message-gateway-update branch from e8cde1d to 6e1c417 Compare April 15, 2022 08:17

rrrooommmaaa added 9 commits April 16, 2022 16:53

Merge remote-tracking branch 'origin/master' into issue-4269-message-…

a5d04c0

…gateway-update

test rendering of recipients after successful sending

25f1bff

lint fix

1f7daf5

testing PWD-encrypted message in reply thread

4460fb3

Merge remote-tracking branch 'origin/master' into issue-4269-message-…

1135878

…gateway-update

lint fix

a4c9c8e

Merge remote-tracking branch 'origin/master' into issue-4269-message-…

1498236

…gateway-update

lint fix

3f8c325

wip

8b91084

rrrooommmaaa added 2 commits June 10, 2022 18:03

added comments

c2c5188

optimizing refactoring

2199b85

tomholub mentioned this pull request Jun 10, 2022

no-explicit-any warnings - prefer unknown #4490

Closed

rrrooommmaaa added 5 commits June 11, 2022 16:24

tslint fix

a327fac

refactored msg formatter

d38ffdd

fix S/MIME drafts

08fc010

updated remarks

f1b2ca8

refactored pwd-related code to a separate method formatttSendablePwdMsgs

cdf57d6

rrrooommmaaa requested a review from tomholub June 12, 2022 11:47

rrrooommmaaa commented Jun 12, 2022

View reviewed changes

extension/chrome/elements/compose-modules/compose-send-btn-module.ts Show resolved Hide resolved

tomholub reviewed Jun 13, 2022

View reviewed changes

tomholub reviewed Jun 20, 2022

View reviewed changes

tomholub mentioned this pull request Jun 20, 2022

refactor pwd messages to only create pgpmime attachments once per batch #4495

Open

Roman Shevchenko added 2 commits June 25, 2022 04:53

separate recipients and attachments to render into a separate renderS…

00d330c

…entMessage object

Merge remote-tracking branch 'origin/master' into issue-4269-message-…

4e52adc

…gateway-update

rrrooommmaaa mentioned this pull request Jun 25, 2022

Adjust send progress when sending multiple messages #4509

Open

Roman Shevchenko and others added 8 commits June 25, 2022 05:03

renamed createPgpMimeAttachments to formatEncryptedMimeDataAsPgpMimeM…

62a826f

…etaAttachments

Use plain sendableNonPwdMsg for pubkey recipients

5cf0652

lint fix

f86d149

corrected legacy pwd message

49fa0f8

fixed test setup

5d77514

Added a test for one legacy pwd recipient and one pubkey recipient of…

35ef46a

… a message

fix

a9978c5

Merge branch 'master' into issue-4269-message-gateway-update

72c7819

tomholub approved these changes Jul 1, 2022

View reviewed changes

tomholub merged commit fae2bb7 into master Jul 9, 2022

tomholub deleted the issue-4269-message-gateway-update branch July 9, 2022 09:42

rrrooommmaaa commented Jul 10, 2022

View reviewed changes

rrrooommmaaa mentioned this pull request Jul 10, 2022

reply sometimes showing [object Object] instead of recipients #4497

Closed

Individual message per recipient #4284

Individual message per recipient #4284

Uh oh!

Conversation

rrrooommmaaa commented Feb 13, 2022 • edited by tomholub Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

To be filled by reviewers

Uh oh!

rrrooommmaaa commented Feb 13, 2022

Uh oh!

rrrooommmaaa commented Feb 17, 2022

Uh oh!

rrrooommmaaa commented Feb 23, 2022

Uh oh!

tomholub commented Mar 7, 2022

Uh oh!

Uh oh!

tomholub commented Mar 24, 2022

Uh oh!

tomholub commented Apr 2, 2022

Uh oh!

rrrooommmaaa commented Apr 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tomholub commented Apr 5, 2022

Uh oh!

tomholub left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rrrooommmaaa commented Jun 12, 2022

Uh oh!

tomholub left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomholub left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tomholub Jun 20, 2022

Choose a reason for hiding this comment

Uh oh!

tomholub left a comment

Choose a reason for hiding this comment

Uh oh!

rrrooommmaaa Jul 10, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rrrooommmaaa commented Feb 13, 2022 •

edited by tomholub

Loading

rrrooommmaaa commented Apr 4, 2022 •

edited

Loading

tomholub left a comment •

edited

Loading