Skip to content

issue #554 add pub keys refresh for contacts#710

Merged
tomholub merged 3 commits intomasterfrom
feature/issue-554-pubkeys-fresh
Oct 19, 2021
Merged

issue #554 add pub keys refresh for contacts#710
tomholub merged 3 commits intomasterfrom
feature/issue-554-pubkeys-fresh

Conversation

@sosnovsky
Copy link
Collaborator

@sosnovsky sosnovsky commented Oct 19, 2021

This PR adds autorefresh for local pub keys.

close #554


Tests (delete all except exactly one):

  • Difficult to test (explain why) - LocalContactsProvider uses Realm, we're currently don't have setup for testing Realm models

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

@sosnovsky sosnovsky requested a review from tomholub October 19, 2021 15:31
@sosnovsky sosnovsky marked this pull request as ready for review October 19, 2021 15:31
@sosnovsky
Copy link
Collaborator Author

@tomholub should we start adding tests for Realm models and services to improve test coverage?

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.

I've removed a few private methods to make it clearer. Looks good, will test.

@tomholub
Copy link
Collaborator

@tomholub should we start adding tests for Realm models and services to improve test coverage?

I'm not too worried about testing individual components - except when they do something complicated (what our db does is pretty simple).

I would rather test larger functionality, to make sure it delivers business value. For example, to test that public keys indeed stay up to date. That's a harder test to write, but more valuable. We'll need a bunch of infrastructure for testing that, that I'll bring over from the browser extension.

@tomholub
Copy link
Collaborator

Works well. I tested the following scenario:

  1. loaded demo@flowcrypt.com in compose
  2. checked Contacts in app
  3. updated EKM to change expiration of the key
  4. loaded demo@flowcrypt.com in compose
  5. checked contacts - correctly showed new expiration 👍
  6. went to EKM to add one more key to demo@flowcrypt.com user
  7. went to compose again and added demo@flowcrypt.com
  8. went to contacts - now showing two keys for demo@flowcrypt.com 👍

We'll write a UI test for this later. For the UI test to be viable, the WKD has to be mocked.

@tomholub tomholub merged commit 34579d5 into master Oct 19, 2021
@tomholub tomholub deleted the feature/issue-554-pubkeys-fresh branch October 19, 2021 17:48
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.

keep public keys fresh

3 participants