Skip to content

Configure sending options prior typing in ComposePageRecipe#5053

Merged
sosnovsky merged 4 commits intomasterfrom
issue-5052-sending-opts-draft-save
Apr 4, 2023
Merged

Configure sending options prior typing in ComposePageRecipe#5053
sosnovsky merged 4 commits intomasterfrom
issue-5052-sending-opts-draft-save

Conversation

@rrrooommmaaa
Copy link
Contributor

This PR first sets up sending options, then types the message, invoking draft save.

close #5052


Tests (delete all except exactly one):

  • internal test refactoring

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 rrrooommmaaa requested a review from sosnovsky as a code owner April 1, 2023 09:23
Copy link
Collaborator

@sosnovsky sosnovsky left a comment

Choose a reason for hiding this comment

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

I tried to reproduce bug which you described in #5052, but it works well for me - Saved label appears even after clicking checkbox for Rich text. Were you able to reproduce it?

Also for this case I think we should fix extension behavior, not change test, as users should see Saved label even when they click sending options checkboxes after composing message.

@rrrooommmaaa
Copy link
Contributor Author

I tried to reproduce bug which you described in #5052, but it works well for me - Saved label appears even after clicking checkbox for Rich text. Were you able to reproduce it?

Yes, in Firefox, after typing something, when "Saved" appears, I switch "Rich Text" from "off" to "on" -- "Saved" gets immediately cleared and doesn't re-appear.
Important: If switching "Rich text" from "on" to "off", then "Saved" appears because this switch results in message re-formatting with draft save.

Also for this case I think we should fix extension behavior, not change test, as users should see Saved label even when they click sending options checkboxes after composing message.

Maybe they should, but switching the options takes some time, and as "Saved" may appear before (or during) the switching, there will be less time left to check that "Saved" is "continuously visible", potentially leading to unstable test.

sosnovsky
sosnovsky previously approved these changes Apr 3, 2023
@rrrooommmaaa
Copy link
Contributor Author

How can I see the actual failed assertion?

@sosnovsky
Copy link
Collaborator

@rrrooommmaaa
Copy link
Contributor Author

We need to extract this meaningful piece into the test result:

  ✘ [fail]: compose - saving and rendering a draft with RTL text (plain text) [ALL RETRIES FAILED for compose - saving and rendering a draft with RTL text (plain text)]
    ℹ (attempt 1 of 1) Failed:   AssertionError: expected '<div><br></div><div><br></div><div>--…' to include '<div dir="rtl">مرحبا<br></div>'
          at <anonymous> (/home/osboxes/flowcrypt-browser/test/source/tests/compose.ts:1508:62)
          at <anonymous> (/home/osboxes/flowcrypt-browser/test/source/test.ts:81:11)
          at BrowserPool.BrowserPool.withNewBrowserTimeoutAndRetry (/home/osboxes/flowcrypt-browser/test/source/browser/browser-pool.ts:113:11)
          at <anonymous> (/home/osboxes/flowcrypt-browser/test/source/test.ts:75:7)
          at <async> <anonymous> (/test/source/tests/compose.ts:1495:39)
          at <async> <anonymous> (../test/source/test.ts:75:55)

The line with (attempt x of x) Failed (last attempt)

await ComposePageRecipe.waitForToastToAppearAndDisappear(composePage, 'Draft not saved: Error: Your account keys are revoked');
await Promise.all([
ComposePageRecipe.fillMsg(composePage, { to: 'mock.only.pubkey@flowcrypt.com' }, 'no valid key'),
ComposePageRecipe.waitForToastToAppearAndDisappear(composePage, 'Draft not saved: Error: Your account keys are revoked'),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This toast appears only once, doesn't reappear on further edits, is this behavior expected?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, it's expected behavior - we set disableSendingDrafts to true in case of such error (as there is no reason to retry draft saving)

https://github.com/FlowCrypt/flowcrypt-browser/blob/master/extension/chrome/elements/compose-modules/compose-draft-module.ts#L183

Copy link
Collaborator

@sosnovsky sosnovsky left a comment

Choose a reason for hiding this comment

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

Well done 👍

@sosnovsky sosnovsky merged commit d6792cd into master Apr 4, 2023
@sosnovsky sosnovsky deleted the issue-5052-sending-opts-draft-save branch April 4, 2023 17:17
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.

Compose test unstable possibly due to sendingOpts

2 participants