feat: add and remove conversation favorite [WPB-11639] #3119

Garzas · 2024-11-21T13:12:44Z

WPB-11639 [Android] User Story for Add to favourites/Remove from favourites

PR Submission Checklist for internal contributors

The PR Title
- conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
- contains a reference JIRA issue number like SQPIT-764
- answers the question: If merged, this PR will: ... ³
The PR Description
- is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

use cases for adding and removing conversation to favorites
syncConversationFoldersFromLocal for updating all folders from local to API
updated slow sync manager version to sync all folders from API
added isFavorite to conversation details view

github-actions · 2024-11-21T13:22:40Z

Test Results

3 327 tests +18 3 220 ✅ +18 5m 1s ⏱️ +18s
569 suites + 4 107 💤 ± 0
569 files + 4 0 ❌ ± 0

Results for commit 8630eb2. ± Comparison against base commit e271934.

♻️ This comment has been updated with latest results.

github-actions · 2024-11-21T13:29:11Z

Bencher Report

Branch	feat/manage-favorite-folder
Testbed	ubuntu-latest

⚠️ WARNING: No Threshold found!
Without a Threshold, no Alerts will ever be generated.
Latency (nanoseconds (ns))
Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds CLI flag.

Click to view all benchmark results

Benchmark	Latency	nanoseconds (ns)
com.wire.kalium.benchmarks.logic.CoreLogicBenchmark.createObjectInFiles	📈 view plot ⚠️ NO THRESHOLD	662,439.36
com.wire.kalium.benchmarks.logic.CoreLogicBenchmark.createObjectInMemory	📈 view plot ⚠️ NO THRESHOLD	613,261,121.28
com.wire.kalium.benchmarks.persistence.MessagesNoPragmaTuneBenchmark.messageInsertionBenchmark	📈 view plot ⚠️ NO THRESHOLD	1,334,407,338.55
com.wire.kalium.benchmarks.persistence.MessagesNoPragmaTuneBenchmark.queryMessagesBenchmark	📈 view plot ⚠️ NO THRESHOLD	22,118,427.98

🐰 View full continuous benchmarking report in Bencher

codecov-commenter · 2024-11-21T13:47:51Z

Codecov Report

Attention: Patch coverage is 85.85859% with 14 lines in your changes missing coverage. Please review.

Project coverage is 54.14%. Comparing base (e271934) to head (8630eb2).

Files with missing lines	Patch %	Lines
...o/conversation/folder/ConversationFolderDAOImpl.kt	79.16%	3 Missing and 2 partials ⚠️
...ire/kalium/logic/data/conversation/Conversation.kt	0.00%	3 Missing ⚠️
...um/network/api/v0/authenticated/PropertiesApiV0.kt	0.00%	3 Missing ⚠️
...um/logic/feature/conversation/ConversationScope.kt	0.00%	2 Missing ⚠️
...lium/logic/data/conversation/ConversationMapper.kt	66.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3119      +/-   ##
===========================================
+ Coverage    54.02%   54.14%   +0.11%     
===========================================
  Files         1247     1249       +2     
  Lines        36176    36272      +96     
  Branches      3658     3658              
===========================================
+ Hits         19545    19639      +94     
  Misses       15220    15220              
- Partials      1411     1413       +2

Files with missing lines	Coverage Δ
.../conversation/folders/ConversationFolderMappers.kt	`65.21% <100.00%> (+5.21%)`	⬆️
...nversation/folders/ConversationFolderRepository.kt	`93.65% <100.00%> (+2.34%)`	⬆️
...sation/folder/AddConversationToFavoritesUseCase.kt	`100.00% <100.00%> (ø)`
...n/folder/RemoveConversationFromFavoritesUseCase.kt	`100.00% <100.00%> (ø)`
...com/wire/kalium/logic/sync/slow/SlowSyncManager.kt	`93.02% <ø> (ø)`
...onversation/ConversationDetailsWithEventsMapper.kt	`92.63% <100.00%> (+0.07%)`	⬆️
...persistence/dao/conversation/ConversationMapper.kt	`97.89% <100.00%> (+0.02%)`	⬆️
...istence/dao/conversation/ConversationViewEntity.kt	`97.91% <100.00%> (+0.04%)`	⬆️
...ao/conversation/folder/ConversationFolderEntity.kt	`100.00% <100.00%> (ø)`
...lium/logic/data/conversation/ConversationMapper.kt	`58.02% <66.66%> (+0.18%)`	⬆️
... and 4 more

... and 3 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e271934...8630eb2. Read the comment docs.

---- 🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests
JS Bundle Analysis - Avoid shipping oversized bundles

datadog-wireapp · 2024-11-21T13:49:35Z

Datadog Report

Branch report: feat/manage-favorite-folder
Commit report: 0013516
Test service: kalium-jvm

✅ 0 Failed, 3220 Passed, 107 Skipped, 36.5s Total Time

saleniuk · 2024-11-21T15:41:07Z

persistence/src/commonMain/db_user/com/wire/kalium/persistence/ConversationDetails.sq

+END AS interactionEnabled,
+CASE WHEN EXISTS (
+        SELECT 1
+        FROM LabeledConversation lc
+        JOIN ConversationFolder cf ON lc.folder_id = cf.id
+        WHERE lc.conversation_id = Conversation.qualified_id
+          AND cf.folder_type = 'FAVORITE'
+    )
+    THEN 1 ELSE 0 END AS isFavorite


suggestion: making multiple nested queries can be expensive.
I would suggest to change it to use joins, like:

LabeledConversation.folder_id IS NOT NULL AS isFavorite FROM Conversation LEFT JOIN ConversationFolder AS FavoriteFolder ON FavoriteFolder.folder_type = 'FAVORITE' LEFT JOIN LabeledConversation ON LabeledConversation.conversation_id = Conversation.id AND LabeledConversation.folder_id = FavoriteFolder.id

First left join filters out and joins only favorite folder, second left join adds given conversation's folders but filters them so that it only adds the one which matches with favorite folder id, so in the end LabeledConversation.folder_id is equal to favorite folder id or NULL if this conversation is not in favorite folder 😄

saleniuk · 2024-11-21T15:42:29Z

persistence/src/commonMain/db_user/com/wire/kalium/persistence/ConversationFolders.sq

@@ -17,6 +17,19 @@ CREATE TABLE LabeledConversation (
      PRIMARY KEY (folder_id, conversation_id)
 );

+getFolderWithConversations:


question: is it supposed to return all folders? Then maybe name it getAllFoldersWithConversations? 🤔

saleniuk · 2024-11-21T15:44:35Z

.../com/wire/kalium/logic/feature/conversation/folder/RemoveConversationFromFavoritesUseCase.kt

@@ -0,0 +1,125 @@
+/*


change: name of the file is RemoveConversationFromFaroritesUseCase without Test at the end 😄

ohassine · 2024-11-22T08:59:34Z

...nMain/kotlin/com/wire/kalium/logic/data/conversation/folders/ConversationFolderRepository.kt

+        return wrapStorageRequest {
+            conversationFolderDAO.addConversationToFolder(conversationId.toDao(), folderId)
+        }
+            .flatMap {
+                syncConversationFoldersFromLocal()
+            }
+    }


I'd suggest to move syncConversationFoldersFromLocal() to the usecase.

Repository should stay as dummy class, it just interacts with data sources by providing CRUD operations and do data transformation.

ohassine · 2024-11-22T08:59:47Z

...nMain/kotlin/com/wire/kalium/logic/data/conversation/folders/ConversationFolderRepository.kt

+        }.flatMap {
+            syncConversationFoldersFromLocal()
+        }


ohassine · 2024-11-22T09:02:43Z

...nMain/kotlin/com/wire/kalium/logic/data/conversation/folders/ConversationFolderRepository.kt

+    override suspend fun syncConversationFoldersFromLocal(): Either<CoreFailure, Unit> {
+        kaliumLogger.withFeatureId(CONVERSATIONS_FOLDERS).v("Syncing conversation folders from local")
+        return wrapStorageRequest { conversationFolderDAO.getFoldersWithConversations().map { it.toModel() } }
+            .flatMap {
+                wrapApiRequest {
+                    userPropertiesApi.updateLabels(
+                        LabelListResponseDTO(it.map { it.toLabel() })
+                    )
+                }
+            }
+    }


This is also breaking the separation of concerns.

We can have this logic in a use case, as it's responsible for orchestrating business logic.

sonarcloud · 2024-11-25T09:43:33Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Garzas added 4 commits November 19, 2024 11:30

feat: manage favorite folder

9581014

Merge branch 'develop' into feat/manage-favorite-folder

1cf021a

feat: add and remove favorite folder

282d0f0

remove unused error class

c8eef8a

Garzas requested review from yamilmedina, MohamadJaara, vitorhugods, saleniuk and damian-kaczmarek November 21, 2024 13:12

Garzas self-assigned this Nov 21, 2024

echoes-hq bot added the echoes: product-roadmap Work aligned with the customer-announced roadmap, targeting a specific release date. label Nov 21, 2024

github-actions bot added 🚨 Potential breaking changes 👕 size: XXL labels Nov 21, 2024

Garzas changed the title ~~Feat/manage favorite folder [WPB-11639]~~ feat: add and remove conversation favorite [WPB-11639] Nov 21, 2024

github-actions bot added the type: feature ✨ label Nov 21, 2024

Merge branch 'develop' into feat/manage-favorite-folder

761f2bc

saleniuk reviewed Nov 21, 2024

View reviewed changes

ohassine reviewed Nov 22, 2024

View reviewed changes

Garzas added 2 commits November 25, 2024 09:48

Merge branch 'develop' into feat/manage-favorite-folder

963fa13

review fixes

8630eb2

Garzas requested review from saleniuk and ohassine November 25, 2024 09:43

ohassine approved these changes Nov 25, 2024

View reviewed changes

Garzas enabled auto-merge November 25, 2024 13:25

saleniuk approved these changes Nov 25, 2024

View reviewed changes

Garzas added this pull request to the merge queue Nov 25, 2024

Merged via the queue into develop with commit c7b96a2 Nov 25, 2024
22 checks passed

Garzas deleted the feat/manage-favorite-folder branch November 25, 2024 14:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add and remove conversation favorite [WPB-11639] #3119

feat: add and remove conversation favorite [WPB-11639] #3119

Garzas commented Nov 21, 2024 •

edited by github-actions bot

Loading

github-actions bot commented Nov 21, 2024 •

edited

Loading

github-actions bot commented Nov 21, 2024 •

edited

Loading

⚠️ WARNING: No Threshold found!

codecov-commenter commented Nov 21, 2024 •

edited

Loading

datadog-wireapp bot commented Nov 21, 2024 •

edited

Loading

saleniuk Nov 21, 2024

saleniuk Nov 21, 2024

saleniuk Nov 21, 2024

ohassine Nov 22, 2024

ohassine Nov 22, 2024

ohassine Nov 22, 2024

sonarcloud bot commented Nov 25, 2024

feat: add and remove conversation favorite [WPB-11639] #3119

feat: add and remove conversation favorite [WPB-11639] #3119

Conversation

Garzas commented Nov 21, 2024 • edited by github-actions bot Loading

PR Submission Checklist for internal contributors

What's new in this PR?

github-actions bot commented Nov 21, 2024 • edited Loading

Test Results

github-actions bot commented Nov 21, 2024 • edited Loading

Bencher Report

⚠️ WARNING: No Threshold found!

codecov-commenter commented Nov 21, 2024 • edited Loading

Codecov Report

datadog-wireapp bot commented Nov 21, 2024 • edited Loading

Datadog Report

saleniuk Nov 21, 2024

Choose a reason for hiding this comment

saleniuk Nov 21, 2024

Choose a reason for hiding this comment

saleniuk Nov 21, 2024

Choose a reason for hiding this comment

ohassine Nov 22, 2024

Choose a reason for hiding this comment

ohassine Nov 22, 2024

Choose a reason for hiding this comment

ohassine Nov 22, 2024

Choose a reason for hiding this comment

sonarcloud bot commented Nov 25, 2024

Quality Gate passed

Garzas commented Nov 21, 2024 •

edited by github-actions bot

Loading

github-actions bot commented Nov 21, 2024 •

edited

Loading

github-actions bot commented Nov 21, 2024 •

edited

Loading

codecov-commenter commented Nov 21, 2024 •

edited

Loading

datadog-wireapp bot commented Nov 21, 2024 •

edited

Loading