Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(contacts): remove hack force refreshing images with a timestamp #17054

Merged
merged 1 commit into from
Jan 21, 2025

Conversation

jrainville
Copy link
Member

What does the PR do

Fixes #16814

This gets rid of the uglu hack of adding addTimestampToURL to each image.

Now, the url coming from status-go contains the timestamp of when the image was updated. The status-go PR is here: status-im/status-go#6239

Affected areas

Contact list, member lists, user profile

Architecture compliance

Screenshot of functionality

profile-image-change.webm

Impact on end user

Nothing. It should work as before. There is just less ugly code and in theory it would be a nonosecond faster

How to test

  • Update your profile picture in one account and check that the other account sees the new image everywhere

Risk

Tick one:

  • Low risk: 2 devs MUST perform testing as specified above and attach their results as comments to this PR before merging.
  • High risk: QA team MUST perform additional testing in the specified affected areas before merging.

Worst case, some place the image doesn't update until a restart

@jrainville jrainville requested review from micieslak, caybro, alexjba and a team as code owners January 10, 2025 15:53
@jrainville jrainville requested a review from a team January 10, 2025 15:53
@status-im-auto
Copy link
Member

status-im-auto commented Jan 10, 2025

Jenkins Builds

Click to see older builds (14)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ f82f0c1 #1 2025-01-10 16:02:58 ~8 min tests/nim 📄log
✔️ f82f0c1 #1 2025-01-10 16:05:13 ~11 min macos/aarch64 🍎dmg
✔️ f82f0c1 #1 2025-01-10 16:06:12 ~12 min tests/ui 📄log
✔️ f82f0c1 #1 2025-01-10 16:09:27 ~15 min macos/x86_64 🍎dmg
✔️ f82f0c1 #1 2025-01-10 16:12:18 ~18 min linux-nix/x86_64 📦tgz
✔️ f82f0c1 #1 2025-01-10 16:14:18 ~20 min linux/x86_64 📦tgz
✔️ f82f0c1 #1 2025-01-10 16:20:37 ~26 min windows/x86_64 💿exe
✔️ 939f001 #2 2025-01-14 14:48:13 ~9 min macos/aarch64 🍎dmg
✔️ 939f001 #2 2025-01-14 14:50:21 ~11 min tests/nim 📄log
✔️ 939f001 #2 2025-01-14 14:51:28 ~12 min macos/x86_64 🍎dmg
✔️ 939f001 #2 2025-01-14 14:57:21 ~18 min tests/ui 📄log
✔️ 939f001 #2 2025-01-14 15:04:34 ~25 min windows/x86_64 💿exe
✔️ 939f001 #2 2025-01-14 15:04:44 ~25 min linux-nix/x86_64 📦tgz
✔️ 939f001 #2 2025-01-14 15:06:43 ~27 min linux/x86_64 📦tgz
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 33628bf #3 2025-01-17 19:01:36 ~7 min macos/aarch64 🍎dmg
✔️ 33628bf #3 2025-01-17 19:02:29 ~8 min tests/nim 📄log
33628bf #3 2025-01-17 19:06:05 ~12 min tests/ui 📄log
✔️ 33628bf #3 2025-01-17 19:06:38 ~12 min macos/x86_64 🍎dmg
✔️ 33628bf #3 2025-01-17 19:13:45 ~20 min linux-nix/x86_64 📦tgz
✔️ 33628bf #3 2025-01-17 19:16:26 ~22 min linux/x86_64 📦tgz
✔️ 33628bf #3 2025-01-17 19:19:03 ~25 min windows/x86_64 💿exe
33628bf #4 2025-01-18 06:44:53 ~11 min tests/ui 📄log
✔️ 9827de5 #4 2025-01-20 15:59:54 ~5 min macos/aarch64 🍎dmg
✔️ 9827de5 #4 2025-01-20 16:02:47 ~8 min tests/nim 📄log
✔️ 9827de5 #5 2025-01-20 16:06:21 ~11 min tests/ui 📄log
✔️ 9827de5 #4 2025-01-20 16:07:30 ~12 min macos/x86_64 🍎dmg
✔️ 9827de5 #4 2025-01-20 16:13:15 ~18 min linux/x86_64 📦tgz
✔️ 9827de5 #4 2025-01-20 16:16:36 ~22 min linux-nix/x86_64 📦tgz
✔️ 9827de5 #4 2025-01-20 16:22:17 ~27 min windows/x86_64 💿exe

Copy link
Member

@caybro caybro left a comment

Choose a reason for hiding this comment

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

Very nice! 👍

@virginiabalducci
Copy link

I tested the PR by updating the profile picture in one account and checking that the other account sees the new image everywhere. This looks good! 👍

@virginiabalducci
Copy link

virginiabalducci commented Jan 18, 2025

I noticed an issue that may be related to this:

  1. change the profile picture
  2. Open a 1:1 chat and send a message
    Result: the profile picture of the sender (the actual user) is shown outside the ring in a square format.

Screenshot 2025-01-18 at 1 01 32 PM

The profile pic looks good in the rest of the places.

Edit: this is actually happening on a master build, and is unrelated to this PR, new issue created: #17089

cc @jrainville

Copy link
Member

@micieslak micieslak left a comment

Choose a reason for hiding this comment

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

I'm happy to see it done in a proper way 🫶

@jrainville jrainville merged commit 9dea479 into master Jan 21, 2025
9 checks passed
@jrainville jrainville deleted the refactor/contact-images-refresh branch January 21, 2025 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor contact images to use a "version" in the local URL to avoid using the addTimestampToURL hack
5 participants