-
Notifications
You must be signed in to change notification settings - Fork 0
Attach tags asynchronously #319
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
Conversation
# Conflicts: # app/src/main/java/to/bitkit/repositories/ActivityRepo.kt
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 🎉
Added a few remarks but nothing blocking.
Tested
- the 2 onchain test cases 🟢
| return@withContext Result.failure(e) | ||
| } | ||
| updateActivitiesMetadata() | ||
| syncTagsMetaData() |
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.
nit: MetaData caps NOK, should be Metadata
| if (db.tagMetadataDao().getAll().isEmpty()) return@withContext | ||
| val lastActivities = getActivities(limit = 10u).getOrNull() ?: return@withContext | ||
| Logger.debug("syncTagsMetaData called") | ||
| lastActivities.forEach { activity -> |
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.
Computation inside forEach block should be parallelized later, when optimising for performance.
| ): Result<Unit> = withContext(bgDispatcher) { | ||
| return@withContext runCatching { | ||
|
|
||
| if (tags.isEmpty()) throw InvalidParameterException("tags must not be empty") |
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.
InvalidParameterException is not intended for such uses, apparently.
Should be replaced by IllegalArgumentException
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 intended to implement IllegalArgumentException. It was probably the autocomplemente, thanks!
Closes #322
Description
Currently, the app only tries to attach pending tags when the event is received, so it may fail if the activity takes to long to be added.
This implementation syncs the pending tags on every activity sync
Preview
onchain-receive.mp4
onchain-send.mp4
ln-receive.mp4
ln-send.mp4
QA Notes
Tests
OnChain
Lightning