Skip to content

Conversation

@rrrooommmaaa
Copy link
Contributor

@rrrooommmaaa rrrooommmaaa commented Mar 26, 2022

This PR binds message gateway id for pwd-encrypted messages

close #4269


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
Copy link
Contributor Author

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

Originally posted by @tomholub in #4284 (comment)

Should I keep this PR draft meanwhile?

@tomholub
Copy link
Collaborator

yup 👍

@tomholub
Copy link
Collaborator

tomholub commented Mar 27, 2022

Actually, the whole time we had FES 2022-05 running, which is the latest version.

Have you tried this smaller PR against a live instance, is it giving you any trouble?

@tomholub
Copy link
Collaborator

If you test it against a live instance and it works as intended, you can mark it as ready for review. Thanks! 👍

Copy link
Collaborator

@tomholub tomholub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - see comment below

}
}
if (msg.externalId) {
this.view.acctServer.messageGatewayUpdate(msg.externalId, msgSentRes.id).catch(Catch.reportErr);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can await this, and put it just after draftDelete. I suppose we could make the two calls in parallel if we want to speed it up a little bit.

@rrrooommmaaa
Copy link
Contributor Author

Actually, the whole time we had FES 2022-05 running, which is the latest version.

Have you tried this smaller PR against a live instance, is it giving you any trouble?

I tried that in early February. I don't quite get it -- should I log in as @flowcrypt.com to connect to the right instance?

@tomholub
Copy link
Collaborator

tomholub commented Mar 27, 2022

I tried that in early February.

ok - that was likely on older instance

I don't quite get it -- should I log in as @flowcrypt.com to connect to the right instance?

correct - https://fes.flowcrypt.com will be used on @flowcrypt.com account. fes.example.com will be used on example.com account if it's running there, etc.

@rrrooommmaaa rrrooommmaaa force-pushed the issue-4269-message-gateway-update-1 branch from 35ce6e1 to 0420488 Compare April 1, 2022 13:43
@rrrooommmaaa rrrooommmaaa marked this pull request as ready for review April 2, 2022 07:46
@rrrooommmaaa rrrooommmaaa enabled auto-merge (squash) April 2, 2022 07:46
@rrrooommmaaa rrrooommmaaa requested a review from tomholub April 2, 2022 07:46
@rrrooommmaaa rrrooommmaaa merged commit 5833d9c into master Apr 2, 2022
@rrrooommmaaa rrrooommmaaa deleted the issue-4269-message-gateway-update-1 branch April 2, 2022 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

send MessageID to FES for easier threading of conversations on Gmail

3 participants