Skip to content

#1813 Drafts functionality#1851

Merged
ioanmo226 merged 71 commits intomasterfrom
feature/issue-1813-drafts
Oct 18, 2022
Merged

#1813 Drafts functionality#1851
ioanmo226 merged 71 commits intomasterfrom
feature/issue-1813-drafts

Conversation

@sosnovsky
Copy link
Collaborator

@sosnovsky sosnovsky commented Oct 5, 2022

This PR adds drafts functionality

close #1813
close #888
close #972
close #887
close #834
close #965
close #963
close #873
close #970
close #691


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

@DenBond7
Copy link
Collaborator

@sosnovsky But I've found a new issue. To reproduce you can use our conversation in Gmail. Please choose any message in the thread and make a reply. The app will create a draft. But the web Gmail client can't edit this draft when I try to open a thread. Actually, the web Gmail client shows that there is a draft in the message list, but doesn't show a draft when I open a thread.

It seems you've missed updating headers in the draft. Please check https://developers.google.com/gmail/api/guides/threads#adding_drafts_and_messages_to_threads

@sosnovsky
Copy link
Collaborator Author

PR is ready for re-review - I fixed mentioned bugs and also updated ui test to include more cases.

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.

See comments below, looks good to me. Tests still not happy? Did they get more troublesome lately, or is that PR related?

@sosnovsky
Copy link
Collaborator Author

See comments below, looks good to me. Tests still not happy? Did they get more troublesome lately, or is that PR related?

There are some random issues with tests after updating semaphore config to use xcode 14 and appium 2 (as appium 1 is deprecated and doesn't support xcode 14). Locally tests work well, so this issue happens only on semaphore, currently trying to find stable fix for it.

@DenBond7
Copy link
Collaborator

I've tested the latest build. It works well. Draft functionality looks good. Just found a few UI issues. What's strange, I can reproduce it only in our last conversation. It can be fixed in another PR, @sosnovsky is up to you.

photo1665729362 (1)
photo1665729362

DenBond7
DenBond7 previously approved these changes Oct 14, 2022
@sosnovsky
Copy link
Collaborator Author

Thanks guys for such detailed testing 👍

@ioanmo226 I pushed fixes for found issues, should work well now
@DenBond7 I emailed you link to the new build with fix for separator issue

@sosnovsky
Copy link
Collaborator Author

@sosnovsky By the way, I think we should have UI test(for last case at above comment) which tests draft send feature

Agree, I'll update current ui test to include this case too.

@DenBond7
Copy link
Collaborator

I emailed you link to the new build with fix for separator issue

@sosnovsky I've tested. My issues were fixed. Thanks!

@sosnovsky
Copy link
Collaborator Author

@ioanmo226 I fixed issues from your list and also updated ui test, please check

@sosnovsky
Copy link
Collaborator Author

@ioanmo226 does it happen every time for you? As I wasn't able to reproduce this bug.

Maybe it happens when first 'save draft` request is still in progress and user taps back button, which triggers another 'save draft' request. I'll try to handle this case, probably by cancelling previous request

@sosnovsky
Copy link
Collaborator Author

@ioanmo226 I pushed fix for duplicated drafts, please check if it works well for you now, thanks

@ioanmo226 ioanmo226 merged commit 98f6a43 into master Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment