fix: 1:1s end up at the bottom of the list after migration [WPB-15452] #3232

saleniuk · 2025-01-16T12:17:49Z

WPB-15452 [Android] 1:1s end up at the bottom of the list after migration

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?

Issues

1:1 conversations, after MLS migration, are moved to the bottom of the conversation list, even if the conversation has some messages sent or received in the meantime.

Causes (Optional)

This is due to the fact that when migrating 1:1, a new conversation with different id is created, so in theory this new one doesn’t have any events thus lastEventTime and last_modified_date are empty (01.01.1970) - that’s why it’s moved to the bottom of the list as the query. It creates confusion and inconsistency in sorting as now, even if a conversation had some messages, which are migrated to the new one.

Solutions

Update last_modified_date when migrating history (messages) - take the value from previous proteus conversation or set a current time otherwise (if there's no previous proteus conversation then it means it's a completely new one so it should be at the top anyway).

Testing

Test Coverage (Optional)

I have added automated test to this contribution

How to Test

Have proteus 1:1, send and receive some messages, migrate to MLS, check whether the conversation is still on the proper position on a list.

PR Post Submission Checklist for internal contributors (Optional)

Wire's Github Workflow has automatically linked the PR to a JIRA issue

PR Post Merge Checklist for internal contributors

If any soft of configuration variable was introduced by this PR, it has been added to the relevant documents and the CI jobs have been updated.

References

https://sparkbox.com/foundry/semantic_commit_messages
https://github.com/wireapp/.github#usage
E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

…pdate-last-modified-date-when-migrating-1on1

sonarqubecloud · 2025-01-16T12:25:42Z

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

github-actions · 2025-01-16T12:34:29Z

Test Results

3 405 tests +2 3 297 ✅ +2 5m 51s ⏱️ +32s
584 suites ±0 108 💤 ±0
584 files ±0 0 ❌ ±0

Results for commit 94d3e5d. ± Comparison against base commit 7611f05.

This pull request removes 1 and adds 3 tests. Note that renamed tests count towards both.

com.wire.kalium.logic.feature.conversation.mls.OneOnOneMigratorTest ‑ givenExistingTeamOneOnOne_whenMigratingToProteus_thenShouldNOTCreateGroupConversation[jvm]

com.wire.kalium.logic.feature.conversation.mls.OneOnOneMigratorTest ‑ givenNoProteusConversation_whenMigratingToMLS_thenUpdateLastModifiedDateToBeCurrentDate[jvm]
com.wire.kalium.logic.feature.conversation.mls.OneOnOneMigratorTest ‑ givenProteusConversation_whenMigratingToMLS_thenUpdateLastModifiedDateFromOriginalConversation[jvm]
com.wire.kalium.logic.feature.conversation.mls.OneOnOneMigratorTest ‑ givenUnassignedOneOnOne_whenMigratingToProteus_thenShouldAssignOneOnOneConversation[jvm]

♻️ This comment has been updated with latest results.

MohamadJaara · 2025-01-16T12:39:31Z

logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/mls/OneOnOneMigrator.kt

+                ).map {
+                    // Emit most recent last modified date of the proteus conversations to pass it to the target conversation
+                    conversationRepository.getConversationById(proteusOneOnOneConversation).getOrNull()?.lastModifiedDate.let {
+                        listOfNotNull(lastModifiedDate, it).maxOrNull()
+                    }
+                }
+            }.map { lastModifiedDate ->
+                // Fallback to current time if not found as it means that it's completely new conversation without any history
+                lastModifiedDate ?: currentInstant()
            }
+        }.flatMap { lastModifiedDate ->
+            conversationRepository.updateConversationModifiedDate(targetConversation, lastModifiedDate)


you can do the same in the query that does the update to avoid updating

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

Line 582 in d54cc28

moveMessages:

not sure whether moveMessages query should have any side effects and we don't update it for every message type so we would need to duplicate the logic from persistMessage and MessageContent.shouldUpdateConversationOrder() into sql, also, I think it would be harder to set current time as iso string directly in sql 🤔

github-actions · 2025-01-16T12:40:48Z

Bencher Report

Branch	fix/update-last-modified-date-when-migrating-1on1
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	microseconds (µs)
com.wire.kalium.benchmarks.logic.CoreLogicBenchmark.createObjectInFiles	📈 view plot ⚠️ NO THRESHOLD	682.54
com.wire.kalium.benchmarks.logic.CoreLogicBenchmark.createObjectInMemory	📈 view plot ⚠️ NO THRESHOLD	549,937.03
com.wire.kalium.benchmarks.persistence.MessagesNoPragmaTuneBenchmark.messageInsertionBenchmark	📈 view plot ⚠️ NO THRESHOLD	1,340,324.88
com.wire.kalium.benchmarks.persistence.MessagesNoPragmaTuneBenchmark.queryMessagesBenchmark	📈 view plot ⚠️ NO THRESHOLD	21,870.84

🐰 View full continuous benchmarking report in Bencher

codecov-commenter · 2025-01-16T12:58:10Z

Codecov Report

Attention: Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.

Project coverage is 54.45%. Comparing base (7611f05) to head (94d3e5d).

Files with missing lines	Patch %	Lines
...logic/feature/conversation/mls/OneOnOneMigrator.kt	90.90%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3232      +/-   ##
===========================================
- Coverage    54.47%   54.45%   -0.03%     
===========================================
  Files         1269     1269              
  Lines        36964    36971       +7     
  Branches      3745     3747       +2     
===========================================
- Hits         20137    20132       -5     
- Misses       15414    15426      +12     
  Partials      1413     1413

Files with missing lines	Coverage Δ
...logic/feature/conversation/mls/OneOnOneMigrator.kt	`79.22% <90.90%> (-13.64%)`	⬇️

... 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 7611f05...94d3e5d. Read the comment docs.

datadog-wireapp · 2025-01-16T12:58:34Z

Datadog Report

Branch report: fix/update-last-modified-date-when-migrating-1on1
Commit report: fd63e6e
Test service: kalium-jvm

✅ 0 Failed, 3297 Passed, 108 Skipped, 1m 1.84s Total Time

fix: 1:1s end up at the bottom of the list after migration [WPB-15452]

81d146f

echoes-hq bot added the echoes: product-roadmap/bug Work contributing to resolve a bug not critical enough to have raised an incident. label Jan 16, 2025

github-actions bot assigned saleniuk Jan 16, 2025

github-actions bot added 🚨 Potential breaking changes type: bug / fix 🐞 👕 size: M labels Jan 16, 2025

saleniuk added 2 commits January 16, 2025 13:24

fix detekt

274f8de

Merge remote-tracking branch 'refs/remotes/origin/develop' into fix/u…

94d3e5d

…pdate-last-modified-date-when-migrating-1on1

MohamadJaara reviewed Jan 16, 2025

View reviewed changes

saleniuk requested a review from MohamadJaara January 16, 2025 12:55

MohamadJaara approved these changes Jan 23, 2025

View reviewed changes

MohamadJaara requested a review from vitorhugods January 24, 2025 11:36

Garzas approved these changes Jan 24, 2025

View reviewed changes

saleniuk added this pull request to the merge queue Jan 24, 2025

Merged via the queue into develop with commit 544ccea Jan 24, 2025
23 checks passed

saleniuk deleted the fix/update-last-modified-date-when-migrating-1on1 branch January 24, 2025 13:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: 1:1s end up at the bottom of the list after migration [WPB-15452] #3232

fix: 1:1s end up at the bottom of the list after migration [WPB-15452] #3232

saleniuk commented Jan 16, 2025 •

edited by github-actions bot

Loading

sonarqubecloud bot commented Jan 16, 2025

github-actions bot commented Jan 16, 2025 •

edited

Loading

MohamadJaara Jan 16, 2025

saleniuk Jan 16, 2025

github-actions bot commented Jan 16, 2025

⚠️ WARNING: No Threshold found!

codecov-commenter commented Jan 16, 2025

datadog-wireapp bot commented Jan 16, 2025

fix: 1:1s end up at the bottom of the list after migration [WPB-15452] #3232

fix: 1:1s end up at the bottom of the list after migration [WPB-15452] #3232

Conversation

saleniuk commented Jan 16, 2025 • edited by github-actions bot Loading

PR Submission Checklist for internal contributors

What's new in this PR?

Issues

Causes (Optional)

Solutions

Testing

Test Coverage (Optional)

How to Test

PR Post Submission Checklist for internal contributors (Optional)

PR Post Merge Checklist for internal contributors

References

sonarqubecloud bot commented Jan 16, 2025

Quality Gate passed

github-actions bot commented Jan 16, 2025 • edited Loading

Test Results

MohamadJaara Jan 16, 2025

Choose a reason for hiding this comment

saleniuk Jan 16, 2025

Choose a reason for hiding this comment

github-actions bot commented Jan 16, 2025

Bencher Report

⚠️ WARNING: No Threshold found!

codecov-commenter commented Jan 16, 2025

Codecov Report

datadog-wireapp bot commented Jan 16, 2025

Datadog Report

saleniuk commented Jan 16, 2025 •

edited by github-actions bot

Loading

github-actions bot commented Jan 16, 2025 •

edited

Loading