feat(community)_: add version to image url to let clients update #6118
Conversation
Jenkins BuildsClick to see older builds (80)
|
jrainville
force-pushed
the
fix/community-image-not-updating
branch
from
dfc4dc8 to
65d153d
Compare
jrainville
changed the base branch from
develop
to
refactor/dont-use-cache-for-image-server
|
@igor-sirotin @osmaczko I rebased this branch on top of #6127 as I refactored the media sever to not use the community cache. As for the version being in the protobuf, I sadly didn't find a good solution. I'm still hoping you guys can think of something 😕 |
|
Patryk gave me a good suggestion:
Using a local map in the manager |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #6118 +/- ##
===========================================
- Coverage 60.95% 60.93% -0.02%
===========================================
Files 828 828
Lines 109721 109747 +26
===========================================
- Hits 66879 66873 -6
- Misses 35014 35039 +25
- Partials 7828 7835 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
|
jrainville
force-pushed
the
fix/community-image-not-updating
branch
2 times, most recently
from
64d4e66 to
16a9843
Compare
jrainville
requested review from
osmaczko,
qfrank,
ilmotta,
igor-sirotin and
caybro
|
I updated the PR to go with Patryk's recommendation. I no longer need the protobuf changes and the version isn't saved anymore. Please let me know what you think 😄 |
jrainville
force-pushed
the
refactor/dont-use-cache-for-image-server
branch
from
d197857 to
064d880
Compare
jrainville
force-pushed
the
fix/community-image-not-updating
branch
from
16a9843 to
b8d2544
Compare
jrainville
force-pushed
the
refactor/dont-use-cache-for-image-server
branch
from
064d880 to
bc4b467
Compare
jrainville
force-pushed
the
fix/community-image-not-updating
branch
from
b8d2544 to
b3659a1
Compare
jrainville
force-pushed
the
refactor/dont-use-cache-for-image-server
branch
from
bc4b467 to
3de005e
Compare
jrainville
force-pushed
the
fix/community-image-not-updating
branch
from
b3659a1 to
47f41c9
Compare
|
FYI @igor-sirotin @osmaczko these two tests seem flaky. They failed on my PR by work locally:
I'll restart them on my PR |
@jrainville yep they're flaky 💯 |
jrainville
force-pushed
the
fix/community-image-not-updating
branch
from
47f41c9 to
a300874
Compare
You don't need to restart unless one of the tests fails three times in a row, which is highly unlikely even for flaky ones. |
btw This only works if a test fails. But if it panics, then there's no retries 😐 |
|
@igor-sirotin @qfrank @ilmotta can I get one more review please. I updated a lot of the old review wasn't up to date |
jrainville
force-pushed
the
fix/community-image-not-updating
branch
from
a300874 to
9c7fd7f
Compare
Yeah, we have exactly the same problem with profile image as well |
Yeah, when I do the same fix for the profile images, I'll do a pass to generalize the functions, if possible. But currently on Desktop, only the community image showed the issue, because we don't have the "hack" of adding a timestamp to the link each time it changes. I'll work on that later in the next milestone if I have time |
Fixes status-im/status-desktop#16688 Since we use the local image server to show the community image, the URL never changes when we update the image, since it's served using a query string containing the community ID. eg: `https://Localhost:46739/communityDescriptionImages?communityID=0x03c5ece7da362d31199fb02d632f85fdf853af57d89c3204b4d1e90c6ec13bb23c&name=thumbnail` Because of that, the clients cannot know if the image was updated, so they had to force update the image every time, which was inefficient. We discovered this issue when I refactored the community client code in Desktop so that we only update the changed properties of a community instead of reseting the whole thing. The solution I came up with in the PR is to add a `version` to the URL when we detect that the image changed. This let's the clients detect when the image was updated without having to do any extra logic.
jrainville
force-pushed
the
fix/community-image-not-updating
branch
from
9c7fd7f to
dd62d1c
Compare
Fixes status-im/status-desktop#16688
Since we use the local image server to show the community image, the URL never changes when we update the image, since it's served using a query string containing the community ID. eg:
https://Localhost:46739/communityDescriptionImages?communityID=0x03c5ece7da362d31199fb02d632f85fdf853af57d89c3204b4d1e90c6ec13bb23c&name=thumbnailBecause of that, the clients cannot know if the image was updated, so they had to force update the image every time, which was inefficient.We discovered this issue when I refactored the community client code in Desktop so that we only update the changed properties of a community instead of reseting the whole thing.
The solution I came up with in the PR is to add a
versionto the URL when we detect that the image changed. This let,s the clients detect when the image was updated without having to do any extra logic.