Skip to content

Fix/interactive message stale image cache key#9903

Open
cevn wants to merge 3 commits into
mattermost:mainfrom
cevn:fix/interactive-message-stale-image-cache-key
Open

Fix/interactive message stale image cache key#9903
cevn wants to merge 3 commits into
mattermost:mainfrom
cevn:fix/interactive-message-stale-image-cache-key

Conversation

@cevn

@cevn cevn commented Jun 30, 2026

Copy link
Copy Markdown

Summary

We have a Mattermost plugin integration which utilizes Mattermost’s “interactive messages” feature. The plugin updates the attachments on a message in-place, specifically the “image_url” attachment, whenever a user clicks an interactive action button on the same message. This image URL refresh had previously been working as expected across all Mattermost platforms (Desktop, Web, and Mobile).

Beginning some time in late 2025 however, the updated image has failed to properly render on Mattermost Mobile, for all posts. Users are currently required to refresh the channel (or simply visit another channel and return to the original channel) to see the updated image.

The image update continues to work as expected on all non-mobile Mattermost platforms, i.e. Desktop and Web versions. We believe that the regression may have been introduced with Mattermost Mobile v2.33.0.

Upon deeper analysis of the Mattermost open-source code, we believe the issue is related to a stale attachment image URL essentially being cached and reused by the client at rendering time. A callback to refresh the Mattermost message continues to use this stale URL value, instead of the updated value provided to the callback.

This pull request makes a change to derive the cache key from the current image URL, whilst utilizing the useMemo method, so that the URL is properly updated upon being changed.

We have tested and verified this change on our own iOS development version of Mattermost Mobile, released via Test Flight.

Affects both render paths: message_attachments/attachment_image and
block_renderer/mm_blocks_image.

Ticket Link

Checklist

  • Added or updated unit tests (required for all new features)
  • Has UI changes
  • Includes text changes and localization file updates
  • Have tested against the 5 core themes to ensure consistency between them.
  • Have run E2E tests by adding label E2E/Run (or E2E/Run-iOS / E2E/Run-Android for platform-specific runs).

Device Information

This PR was tested on:

Screenshots

Release Note


sdhar and others added 2 commits June 29, 2026 18:52
The attachment image components froze their expo-image cacheKey (fileId)
to the first imageUrl via useRef. On an in-place post edit (same post id,
e.g. interactive-message button updates) the new image URL was rendered
under the old cacheKey, so expo-image kept serving the originally cached
image and never refreshed. Derive the cacheKey from the current imageUrl
with useMemo so it updates when the URL changes.

Affects both render paths: message_attachments/attachment_image and
block_renderer/mm_blocks_image.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Asserts the expo-image cache id (fileId) tracks imageUrl across re-renders,
so an in-place post edit to a new image URL is not served from the stale
cache. Fails on the previous useRef implementation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mattermost-build

Copy link
Copy Markdown
Contributor

Hello @cevn,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

Per the Mattermost Contribution Guide, we need to add you to the list of approved contributors for the Mattermost project.

Please help complete the Mattermost contribution license agreement?
Once you have signed the CLA, please comment with /check-cla and confirm that the CLA check is green.

This is a standard procedure for many open source projects.

Please let us know if you have any questions.

We are very happy to have you join our growing community! If you're not yet a member, please consider joining our Contributors community channel to meet other contributors and discuss new opportunities with the core team.

@mm-cloud-bot mm-cloud-bot added the kind/bug Categorizes issue or PR as related to a bug. label Jun 30, 2026
@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Two components switch fileId derivation from mutable refs to memoized values from imageUrl, and use that value consistently for gallery payloads and image identifiers. AttachmentImage also gains tests for cache ID updates, gallery opening, and invalid URL handling.

Changes

fileId memoization and gallery caching

Layer / File(s) Summary
mm_blocks_image fileId memoization
app/components/block_renderer/mm_blocks_image.tsx
fileId is computed via useMemo from imageUrl instead of useRef; gallery item id/cacheKey, the useCallback dependency list, and ExpoImage id/nativeID props now reference the memoized value.
AttachmentImage fileId memoization
app/components/post_list/post/body/content/message_attachments/attachment_image/index.tsx
useRef import removed; fileId is now useMemo-derived from imageUrl and used for the onPress gallery item id/cacheKey and the ProgressiveImage id prop.
AttachmentImage test suite
app/components/post_list/post/body/content/message_attachments/attachment_image/index.test.tsx
New tests mock progressive image, gallery, and URL validation modules, then verify ProgressiveImage cache id derivation, cache id updates on imageUrl change, openGalleryAtIndex payload correctness, and skipped rendering for invalid imageUrl.

Sequence Diagram(s)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

2: Dev Review, 3: QA Review

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: fixing a stale image cache key for interactive message images.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The description matches the code changes by describing the stale image cache key fix, useMemo-based URL derivation, and affected image render paths.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@app/components/post_list/post/body/content/message_attachments/attachment_image/index.test.tsx`:
- Around line 113-124: The test in attachment_image/index.test.tsx only verifies
that the expected gallery item is present via openGalleryAtIndex and
expect.arrayContaining, so extra duplicate items could slip through. Update the
assertion around openGalleryAtIndex to also check the gallery items array length
is exactly 1 while keeping the existing item shape checks for the
IMAGE_URL-derived entry.
- Around line 78-130: The test descriptions in AttachmentImage’s spec do not
follow the required it('should...') convention. Update the four test names in
the AttachmentImage test block to start with “should” while keeping the existing
assertions and behavior unchanged, and make sure the renamed cases still clearly
describe ProgressiveImage rendering, cache id updates on rerender, gallery
opening, and invalid URL handling.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e6888956-9758-4181-8c9b-a2f31d56307f

📥 Commits

Reviewing files that changed from the base of the PR and between 84ea188 and a7970d7.

📒 Files selected for processing (3)
  • app/components/block_renderer/mm_blocks_image.tsx
  • app/components/post_list/post/body/content/message_attachments/attachment_image/index.test.tsx
  • app/components/post_list/post/body/content/message_attachments/attachment_image/index.tsx

- Rename the four AttachmentImage tests to the repo's it('should...') convention
- Assert the gallery items array length is exactly 1 (not just contains)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@cevn

cevn commented Jul 11, 2026

Copy link
Copy Markdown
Author

/check-cla

@cevn

cevn commented Jul 21, 2026

Copy link
Copy Markdown
Author

@saturninoabril or @amyblais can you take a look and lmk if anything needs to be done to merge this? It fixes a real bug for us on the mobile app regarding edited integration posts with image attachments.

@amyblais

Copy link
Copy Markdown
Member

can you take a look and lmk if anything needs to be done to merge this? It fixes a real bug for us on the mobile app regarding edited integration posts with image attachments.

I'll add some reviewers to the PR.

@amyblais amyblais added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Jul 22, 2026 — with Claude
@amyblais
amyblais requested review from enahum and larkox July 22, 2026 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester Contributor kind/bug Categorizes issue or PR as related to a bug. release-note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants