Extended notification preview #6927
Conversation
charlag
force-pushed
the
notification-preview-6608-4
branch
from
d84951d
to
7e1bdcf
Compare
| private val authenticationPrompt: AuthenticationPrompt, | ||
| ) : NativeCredentialsFacade { | ||
| private val keyStoreFacade: AndroidKeyStoreFacade, | ||
| private val activity: Context, |
There was a problem hiding this comment.
Why was this changed to Context? It still being used as a FragmentActivity
There was a problem hiding this comment.
Because this is also used from a JobService alongside the MainActivity. The as FragmentActivity ones are not called from the service. We can maybe refactor this later, for now it needs to use Context to match both.
| if (encryptionMode == CredentialEncryptionMode.BIOMETRICS) { | ||
| promptInfoBuilder.setNegativeButtonText(activity.getString(android.R.string.cancel)) | ||
| } | ||
| val promptInfo = promptInfoBuilder.build() | ||
| authenticationPrompt.authenticateCryptoObject(activity, promptInfo, cryptoObject) | ||
| authenticationPrompt.authenticateCryptoObject(activity as FragmentActivity, promptInfo, cryptoObject) |
| if (login != other.login) return false | ||
| if (type != other.type) return false | ||
| if (userId != other.userId) return false | ||
| if (encryptedPassword != other.encryptedPassword) return false |
There was a problem hiding this comment.
Wouldn't be better to merge these four if statements in just one?
| result = 31 * result + type.hashCode() | ||
| result = 31 * result + userId.hashCode() | ||
| result = 31 * result + encryptedPassword.hashCode() | ||
| result = 31 * result + (databaseKey?.contentHashCode() ?: 0) | ||
| result = 31 * result + accessToken.contentHashCode() |
There was a problem hiding this comment.
Is there a specific reason, other than the standard hashCode implementation, for using 31?
There was a problem hiding this comment.
I think it comes from server code. Need to ask @charlag exactly why though.
There was a problem hiding this comment.
that's what IDEA is generating. We need impl here because we have by arrays
| public func deleteByUserId(_ id: String) async throws { try self.credentialsDb.delete(userId: id) } | ||
| public func getCredentialEncryptionMode() async throws -> CredentialEncryptionMode? { try self.credentialsDb.getCredentialEncryptionMode() } |
| func getExtendedNotificationConfig(_ userId: String) async throws -> TutanotaSharedFramework.ExtendedNotificationMode { | ||
| try self.notificationStorage.getExtendedNotificationConfig(userId) | ||
| } | ||
|
|
There was a problem hiding this comment.
async without await
| let systemPasswordSupported = self.appLockHandler.isSystemPasswordSupported() | ||
| if systemPasswordSupported { supportedMethods.append(.system_pass_or_biometrics) } | ||
| let biometricsSupported = self.appLockHandler.isBiometricsSupported() | ||
| if biometricsSupported { supportedMethods.append(.biometrics) } |
There was a problem hiding this comment.
Could it be simplified to if let ... =?
| ) {} | ||
|
|
||
| async onMailNotification(sseInfo: SseInfo, notificationInfo: NotificationInfo) { | ||
| const w = this.wm.getAll().find((w) => w.getUserId() === notificationInfo.userId) |
There was a problem hiding this comment.
Would be nice to have a better name instead of just w and wm. Maybe mainWindow and winManager?
| internal = "internal", | ||
| external = "external", |
There was a problem hiding this comment.
We should use PascalCase or UPPER_SNAKE_CASE as they seem to be our standard
Implemented extended notifications which include the sender. Re-implemented credential encryption and storage, moved credential data to the native storage, changed credential encryption on mobile to always be device lock + implement app lock independently of encryption. Re-implemented SSE on desktop in a more modular way. Re-organized iOS app to share the code between the main app code and app extensions. Close #6608 Co-authored-by: wec43 <wec@tutao.de>
wec43
force-pushed
the
notification-preview-6608-4
branch
2 times, most recently
from
c2002f5
to
d3774c6
Compare
wec43
force-pushed
the
notification-preview-6608-4
branch
from
d3774c6
to
7fac9d9
Compare
wec43
force-pushed
the
notification-preview-6608-4
branch
from
8bb19f9
to
16daa99
Compare
wrdhub
force-pushed
the
notification-preview-6608-4
branch
from
46b2bf6
to
1dba6e9
Compare
Implemented extended notifications which include the sender.
Re-implemented credential encryption and storage, moved credential data to the native storage, changed credential encryption on mobile to always be device lock + implement app lock independently of encryption.
Re-implemented SSE on desktop in a more modular way.
Re-organized iOS app to share the code between the main app code and app extensions.
Close #6608
tutadb!614
Test notes
Everything needs to be tested on all three platforms:
notificationwithnotification contentandPush IdentifierONLY_SENDERNO_SENDER_OR_SUBJECT, notification title isNew email receivedand body is recipient addressAutomatic unlockandApp passwordAutomatic unlock,System password or biometricsandBiometricsSystem password or biometricsorBiometrics, password or biometrics needs to be verified before login and unlock