Skip to content

Gmail conversation view - #657

Merged
tomholub merged 32 commits into
masterfrom
feature/issue-6080-gmail-conversation
Nov 4, 2021
Merged

Gmail conversation view#657
tomholub merged 32 commits into
masterfrom
feature/issue-6080-gmail-conversation

Conversation

@Kharchevskyi

@KharchevskyiKharchevskyi commented Oct 10, 2021

Copy link
Copy Markdown
Contributor

This PR add conversation view for gmail

close#608
close#725


Tests :

  • Does not need tests (refactor only, docs or internal changes)
  • Difficult to test (explain why)
  • Not worth testing
  • Tests will be added later (issue #...)
  • Tests added or updated

Manual testing:
Imap/Gmail

  • InboxViewController sort order.
  • InboxViewController tap on cell.
  • mark as read
  • mark as unread
  • make sure list is updated after chaning read flag
  • over 5 mb
  • search

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

@KharchevskyiKharchevskyi mentioned this pull request Oct 10, 2021
7 tasks
@Kharchevskyi

Copy link
Copy Markdown
ContributorAuthor

Inbox
s1

@tomholub

Copy link
Copy Markdown
Collaborator

looks good so far

@Kharchevskyi

Copy link
Copy Markdown
ContributorAuthor

s1

@Kharchevskyi

Copy link
Copy Markdown
ContributorAuthor

@tomholub is it ok to show snippet as a subtitle? or it's better to show subject for thread
Also in thread detail controller should I show message snippet for each message in thread? or sender and time would be enough?

@tomholub

Copy link
Copy Markdown
Collaborator

is it ok to show snippet as a subtitle? or it's better to show subject for thread

Subject is more appropriate. You can see yourself, half of the messages are encrypted - like that, user would never be able to find the conversation they are looking for.

Also in thread detail controller should I show message snippet for each message in thread? or sender and time would be enough?

Let's start with sender and time, then re-evaluate.

@Kharchevskyi

Kharchevskyi commented Oct 13, 2021

Copy link
Copy Markdown
ContributorAuthor

Inbox
s1

Thread details
s1

@Kharchevskyi

Copy link
Copy Markdown
ContributorAuthor

Simulator Screen Shot - iPhone 12 mini - 2021-10-17 at 00 14 30

@tomholubtomholub mentioned this pull request Oct 18, 2021
5 tasks
@ivan-ushakov

Copy link
Copy Markdown
Contributor

For example, this file was removed from current master branch:

FlowCrypt/Controllers/MessageList Extension/MsgListViewConroller.swift

But in this branch it is used across several view controllers

@ivan-ushakov

Copy link
Copy Markdown
Contributor

Now I see that it was typo fix in view controller's name:

flowcrypt-ios/FlowCrypt/Controllers/MessageList Extension/MsgListViewConroller.swift
flowcrypt-ios/FlowCrypt/Controllers/MessageList Extension/MsgListViewController.swift

Let me continue with merge

@tomholub

Copy link
Copy Markdown
Collaborator

I know this won't be easy - please see how far you can get.

Comment threadFlowCrypt/Controllers/Compose/ComposeViewController.swift Outdated
Comment threadFlowCrypt/Controllers/Compose/ComposeViewController.swift Outdated
Comment threadFlowCrypt/Controllers/MessageList Extension/MsgListViewController.swift Outdated
Comment threadFlowCrypt/Controllers/Compose/ComposeViewController.swift
@ivan-ushakov

Copy link
Copy Markdown
Contributor

Do we need this in console? Looks like trace level to me:

"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[Core][KeyMethods] pass phrase matches for key: CD7F6CD5BB6B0001C33E1A730DC6E2AAF9533212"
"ℹ️[PassPhraseService] memory: saving passphrase for key CD7F6CD5BB6B0001C33E1A730DC6E2AAF9533212"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft true"

@tomholub

Copy link
Copy Markdown
Collaborator

Do we need this in console? Looks like trace level to me:

"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[Core][KeyMethods] pass phrase matches for key: CD7F6CD5BB6B0001C33E1A730DC6E2AAF9533212"
"ℹ️[PassPhraseService] memory: saving passphrase for key CD7F6CD5BB6B0001C33E1A730DC6E2AAF9533212"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft false"
"ℹ️[ComposeViewController] Draft. Should save draft check"
"ℹ️[ComposeViewController] Draft. Should save draft true"

Agree, the draft logs should be removed. Someone may add them back just for debugging a particular thing if they need to, but not useful to keep in genera.

@tomholubtomholub left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looking good so far, dots below are just to highlight

Comment threadFlowCrypt/Controllers/Compose/ComposeViewController.swift Outdated
Comment threadFlowCrypt/Controllers/Compose/ComposeViewController.swift Outdated
onCompletion(MessageAction.markAsRead(false), .init(message: self.input.objMessage))
navigationController?.popViewController(animated: true)
} catch {
showToast("Could not mark message as unread: \(error)")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

maybe handleOpErr(operation: .markUnread) for consistency?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Better to ask @Kharchevskyi

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

He's not available for the next few days. Can file an issue for later.

Comment threadFlowCrypt/Functionality/Services/GeneralConstants.swift Outdated
Comment threadFlowCrypt/Controllers/MessageList Extension/MsgListViewController.swift Outdated
Comment threadFlowCrypt/Controllers/MessageList Extension/MsgListViewController.swift Outdated
@ivan-ushakov

Copy link
Copy Markdown
Contributor

@tomholub
Test CheckEncryptedEmailAfterRestartApp.spec.ts failing because we use different text in alert:

Simulator Screen Shot - appiumTest-9D7CBD23-6790-436E-926D-CA189C5C5DAC-iPhone X - 2021-11-04 at 13 26 25

but test expects:

[iPhone Simulator iOS 15.0 #0-2] 1) INBOX: user is able to see encrypted email with pass phrase after restart app
[iPhone Simulator iOS 15.0 #0-2] Error: element ("-ios class chain:**/XCUIElementTypeStaticText[`label == "Wrong pass phrase, please try again"`]") still not displayed after 15000ms

@tomholub

Copy link
Copy Markdown
Collaborator

If this was indeed a retry (we already tried pass phrase in the test once and it was wrong, and now we are retrying) than the test is correct, meaning the implementation should be adjusted to match the test.

@ivan-ushakov

ivan-ushakov commented Nov 4, 2021

Copy link
Copy Markdown
Contributor

If this was indeed a retry (we already tried pass phrase in the test once and it was wrong, and now we are retrying) than the test is correct, meaning the implementation should be adjusted to match the test.

We don't have wrong_passphrase in JavaScript code, so we can not get .wrongPassphrase

export enum DecryptErrTypes {
keyMismatch = 'key_mismatch',
usePassword = 'use_password',
wrongPwd = 'wrong_password',
noMdc = 'no_mdc',
badMdc = 'bad_mdc',
needPassphrase = 'need_passphrase',
format = 'format',
other = 'other',
}

@tomholub

Copy link
Copy Markdown
Collaborator

If this was indeed a retry (we already tried pass phrase in the test once and it was wrong, and now we are retrying) than the test is correct, meaning the implementation should be adjusted to match the test.

We don't have wrong_passphrase in JavaScript code, so we can not get .wrongPassphrase

export enum DecryptErrTypes {
keyMismatch = 'key_mismatch',
usePassword = 'use_password',
wrongPwd = 'wrong_password',
noMdc = 'no_mdc',
badMdc = 'bad_mdc',
needPassphrase = 'need_passphrase',
format = 'format',
other = 'other',
}

See how the code behaves on master: handlePassPhraseEntry

privatefunc handlePassPhraseEntry(rawMimeData:Data, with passPhrase:String){showSpinner("loading_title".localized, isUserInteractionEnabled:true)Task{do{letmatched=tryawait serviceActor.checkAndPotentiallySaveEnteredPassPhrase(passPhrase)if matched {
processedMessage =tryawait serviceActor.decryptAndProcessMessage(mime: rawMimeData)handleReceivedMessage()}else{handleWrongPathPhrase(for: rawMimeData, with: passPhrase)}}catch{handleError(error)}}}

the code should work the same on this branch. The first dialog gets shown phrase the first time (originally based on .needPassPhrase - not shown above), and produces a pass phrase dialog. User enters pass phrase. That gets handled by the code above, which will check if the pass phrase is good and produce another dialog, which will be the "wrong pass phrase" dialog. And again until user cancels or good pass phrase is entered.

@ivan-ushakov
ivan-ushakov marked this pull request as ready for review November 4, 2021 13:59

@tomholubtomholub left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks! Still need to manually run this to confirm.

@tomholubtomholub changed the title [update to master] Gmail conversation viewGmail conversation viewNov 4, 2021

@tomholubtomholub left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you Ivan for getting it up to master!

I'll merge it but there will still be some fixes to do after:

  • download percentage is not getting rendered anymore - I think
  • with a very long subject, the number of messages in thread gets hidden because it's at the end

@tomholub
tomholub merged commit ca46d40 into masterNov 4, 2021
@tomholub
tomholub deleted the feature/issue-6080-gmail-conversation branch November 4, 2021 17:00
Sign up for freeto 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.

MessageOperationsProvider move to async await support conversation view in inbox

3 participants

@Kharchevskyi@tomholub@ivan-ushakov