feat: sort tags by alphabetical order (WPB-21606) #4549
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #4549 +/- ##
========================================
Coverage 48.50% 48.50%
========================================
Files 575 575
Lines 19810 19810
Branches 3311 3311
========================================
Hits 9609 9609
Misses 9187 9187
Partials 1014 1014
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
| viewModelScope.launch { | ||
| getAllTagsUseCase().onSuccess { tags -> | ||
| _state.update { it.copy(allTags = tags) } | ||
| _state.update { it.copy(allTags = tags.toList().sorted()) } |
There was a problem hiding this comment.
question: Does it makes sense to return the sorted tags from the use case already ?
There was a problem hiding this comment.
We changed the return type in the usecase to avoid duplicated tags
https://github.com/wireapp/kalium/pull/3572/changes
There was a problem hiding this comment.
I think there is no need to call "toList().sorted()". "sorted()" should just be enough.
| viewModelScope.launch { | ||
| getAllTagsUseCase().onSuccess { tags -> | ||
| _state.update { it.copy(allTags = tags) } | ||
| _state.update { it.copy(allTags = tags.toList().sorted()) } |
There was a problem hiding this comment.
I think there is no need to call "toList().sorted()". "sorted()" should just be enough.
| val addedTags: Set<String> = emptySet(), | ||
| val suggestedTags: Set<String> = emptySet(), | ||
| val allTags: Set<String> = emptySet(), | ||
| val allTags: List<String> = emptyList(), |
There was a problem hiding this comment.
Is it possible to keep set here? All tag collections (added, suggested) are defined as Set, I think we should keep this for allTags as well.
There was a problem hiding this comment.
By default, a Set does not preserve the order of its elements
|
https://wearezeta.atlassian.net/browse/WPB-21606
PR Submission Checklist for internal contributors
The PR Title
SQPIT-764The PR Description
What's new in this PR?
Issues
Sort tags by alphabetical order
Needs releases with:
Testing
Test Coverage (Optional)
How to Test
Briefly describe how this change was tested and if applicable the exact steps taken to verify that it works as expected.
Notes (Optional)
Specify here any other facts that you think are important for this issue.
Attachments (Optional)
Attachments like images, videos, etc. (drag and drop in the text box)
PR Post Submission Checklist for internal contributors (Optional)
PR Post Merge Checklist for internal contributors
References
feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.