Skip to content

fix: move message asset paths to conversation-scoped cache VM [WPB-25002]#4760

Merged
Garzas merged 6 commits into
developfrom
fix/conversation-asset-path-cache
Apr 29, 2026
Merged

fix: move message asset paths to conversation-scoped cache VM [WPB-25002]#4760
Garzas merged 6 commits into
developfrom
fix/conversation-asset-path-cache

Conversation

@Garzas
Copy link
Copy Markdown
Contributor

@Garzas Garzas commented Apr 24, 2026

https://wearezeta.atlassian.net/browse/WPB-25002


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

  • Single-asset messages could get stuck in loading/downloading when switching conversations quickly.
  • Local asset paths were being lost because state was tied to short-lived per-item lifecycle.
  • Loading overlays could still appear even when a valid local image source existed.

Causes (Optional)

  • Asset path state was stored per item, so detach/reattach could recreate state with null path.
  • UI loading decisions relied too much on transfer status instead of source availability.

Solutions

  • Moved message-list asset path handling to a conversation-scoped cache ViewModel (messageId -> localPath) and updated image/video/asset rendering to use it.
  • Updated image overlay/render logic to prioritize available local sources and prevent unnecessary loading placeholders.
  • Added unit tests for the new conversation-scoped asset path flow, including regression coverage for in-progress transfer scenarios.

@Garzas Garzas self-assigned this Apr 24, 2026
@Garzas Garzas changed the title fix: move message asset paths to conversation-scoped cache VM fix: move message asset paths to conversation-scoped cache VM [WPB-25002] Apr 24, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 24, 2026

Codecov Report

❌ Patch coverage is 55.00000% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.80%. Comparing base (67d21d4) to head (16becde).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
...s/messages/item/ConversationAssetPathsViewModel.kt 59.45% 8 Missing and 7 partials ⚠️
...ndroid/ui/home/conversations/model/MessageTypes.kt 0.00% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #4760   +/-   ##
========================================
  Coverage    50.79%   50.80%           
========================================
  Files          604      605    +1     
  Lines        20847    20887   +40     
  Branches      3358     3370   +12     
========================================
+ Hits         10590    10612   +22     
- Misses        9265     9276   +11     
- Partials       992      999    +7     
Files with missing lines Coverage Δ
...ndroid/ui/home/conversations/model/MessageTypes.kt 14.28% <0.00%> (-10.72%) ⬇️
...s/messages/item/ConversationAssetPathsViewModel.kt 59.45% <59.45%> (ø)

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 67d21d4...16becde. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Garzas Garzas force-pushed the fix/conversation-asset-path-cache branch from 6748248 to 7ce9008 Compare April 27, 2026 06:15
viewModel.resolveIfNeeded(
val messageId = message.header.messageId
val localAssetPath = conversationAssetPathsViewModel.localAssetPath(messageId)
LaunchedEffect(messageId, assetStatus) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we move these 'LaunchEffect' blocks into ViewModel? Like this:

    override fun localAssetPath(conversationId: ConversationId, messageId: String, assetStatus: AssetTransferStatus?): String? = localAssetPaths[messageId].also { path ->
        if (path == null && resolvingJobs[messageId]?.isActive != true) {
            resolveIfNeeded(
                conversationId = conversationId,
                messageId = messageId,
                transferStatus = assetStatus ?: AssetTransferStatus.NOT_DOWNLOADED,
                downloadIfNeeded = true,
            )
        }
    }

@sonarqubecloud
Copy link
Copy Markdown

@Garzas Garzas added this pull request to the merge queue Apr 29, 2026
Merged via the queue into develop with commit 886c00e Apr 29, 2026
15 checks passed
@Garzas Garzas deleted the fix/conversation-asset-path-cache branch April 29, 2026 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants