-
-
Notifications
You must be signed in to change notification settings - Fork 594
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add CryptoApi.getBackupInfo
#4512
Add CryptoApi.getBackupInfo
#4512
Conversation
2d5b1fb
to
bd75c71
Compare
…ypto-api-getbackupinfo
1103cf0
to
3a17421
Compare
34f63bc
to
34f9785
Compare
34f9785
to
2a19422
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine, but Rich will know the context.
@andybalaam Yep thx! I'm waiting for his opinion about it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needs better documentation please!
src/crypto-api/index.ts
Outdated
@@ -516,6 +516,14 @@ export interface CryptoApi { | |||
*/ | |||
isKeyBackupTrusted(info: KeyBackupInfo): Promise<BackupTrustInfo>; | |||
|
|||
/** | |||
* Get information about the current key backup. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should define what "current" means here. The version we are currently using for automatic backup? The version we are currently using for automatic restore? The most recent backup on the server? Most recent on the server when we last checked? What if we haven't checked yet? Does it depend on whether we "trusted" the backup or not? How does it relate to getActiveSessionBackupVersion
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I copied the documentation of https://github.com/matrix-org/matrix-js-sdk/blob/develop/src/rust-crypto/backup.ts#L111 since we are just calling this function.
…ypto-api-getbackupinfo
…ypto-api-getbackupinfo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Checklist
public
/exported
symbols have accurate TSDoc documentation.Task element-hq/element-web#26922
Deprecated #4505 and we don't have any equivalent in the cryptoApi.
Add
CryptoApi.getBackupInfo
which is returning the key backup info the current key backup.I wrongly advised to use
CryptoApi.getActiveSessionBackupVersion
which is returning the key backup version while we are expecting a key backup info.Test for the old crypto is useless but I need it to reach more than 80% coverage.