Skip to content

Feature/issue 553 multiple recipient keys - #676

Merged
tomholub merged 16 commits into
masterfrom
feature/issue-553-multiple-recipient-keys
Oct 16, 2021
Merged

tomholub merged 16 commits into
masterfrom
feature/issue-553-multiple-recipient-keys

Conversation

@sosnovsky

@sosnovsky sosnovsky commented Oct 14, 2021

Copy link
Copy Markdown
Collaborator

This PR changes scheme to store multiple public keys per contacts.
UI also updated to show all available contact public keys + new screen added for contact key details.

close #553
close #337


Tests (delete all except exactly one):

  • Tests added or updated - updated tests to use new scheme with multiple contact keys

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

@tomholub tomholub 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.

Early feedback - didn't have a chance to review all yet

Comment thread FlowCrypt/Functionality/Services/Contacts Services/LocalContactsProvider.swift Outdated

@tomholub tomholub 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! Some more / updated comments.

Comment thread FlowCrypt/Functionality/Services/Contacts Services/Models/ContactKey.swift Outdated
Comment thread FlowCrypt/Functionality/Services/Contacts Services/Models/ContactKey.swift Outdated
Comment thread FlowCrypt/Functionality/WKDURLs/PubLookup.swift Outdated
@sosnovsky
sosnovsky marked this pull request as ready for review October 15, 2021 20:14

@tomholub tomholub 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.

Very good. Please see comments. Haven't tried to run it yet.

Please also see if you are able to run the Appium tests after a change like this. I'll send you the env file. Thanks!

Comment thread FlowCrypt/Functionality/WKDURLs/PubLookup.swift
Comment thread FlowCrypt/Models/Realm Models/ContactKeyObject.swift
Comment thread FlowCrypt/Models/Realm Models/ContactKeyObject.swift
Comment thread FlowCrypt/Models/Realm Models/ContactObject.swift
@tomholub

tomholub commented Oct 15, 2021

Copy link
Copy Markdown
Collaborator

@fcvakintos please send me and also roma@ an email with the prepared env file so we can test bigger changes against appium tests. Thanks!

@tomholub

Copy link
Copy Markdown
Collaborator

Did manual testing, looks good - I'll merge it first, please address the PR comments in a follow-up PR.

@tomholub
tomholub enabled auto-merge (squash) October 16, 2021 19:37
@tomholub
tomholub merged commit c498b49 into master Oct 16, 2021
@tomholub
tomholub deleted the feature/issue-553-multiple-recipient-keys branch October 16, 2021 19:52
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.

contacts store should allow more than one public key per recipient email address PubLookup.lookup to return an array of public keys

2 participants