[ABW-3847] Integrate sargon logic for security problems in Android wallet #1274
Conversation
| ) | ||
| } | ||
|
|
||
| sargonOsManager.sargonOs.checkSecurityProblems( |
There was a problem hiding this comment.
Both sargonOs getter and checkSecurityProblems method on it are throwing. Please ensure the exception is properly handled.
There was a problem hiding this comment.
yes good catch! thank you
There was a problem hiding this comment.
@sergiupuhalschi-rdx what do you recommend about handling the exception?
adding a runCatching in the usecase and return Flow<Result<Set<SecurityProblem>>>
or adding a .catch { Timber.e("failed to get security problems...") } in the viewmodel where I collect the flows?
I’m leaning toward the second option, tbh.
There was a problem hiding this comment.
If we should just propagate the error with no additional logic then catch looks like a better option. You might also consider adding catch inside the useCase instead of each viewModel using this useCase. Either option sounds good.
| }.collect {} | ||
| } | ||
| .flowOn(defaultDispatcher) | ||
| .collect {} |
There was a problem hiding this comment.
To prevent unnecessary instantiating a FlowCollector, there is a collect method with no params.
| unrecoverableEntities = findUnrecoverableEntities(entitiesWithSecurityPrompts), | ||
| withoutControlEntities = findWithoutControlEntities(entitiesWithSecurityPrompts), | ||
| lastCloudBackup = BackupResult( | ||
| saveIdentifier = "string", |
There was a problem hiding this comment.
Is this a dummy text because the field is only used by iOS? Maybe we'd benefit from having a comment here stating this if this is the case.
There was a problem hiding this comment.
forgot to update this.
I will pass the google drive file id there.
There was a problem hiding this comment.
@sergiupuhalschi-rdx removed
|
giannis-rdx
deleted the
feature/ABW-3847-integrate-sargon-logic-for-security-problems
branch
Description
read this
How to test
PR submission checklist