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

Update file paths in content when media file is replaced #1168

Closed
Tracked by #1704
timobrembeck opened this issue Feb 15, 2022 · 5 comments · Fixed by #2705
Closed
Tracked by #1704

Update file paths in content when media file is replaced #1168

timobrembeck opened this issue Feb 15, 2022 · 5 comments · Fixed by #2705
Assignees
Labels
‼️ prio: high Needs to be resolved ASAP. 🐛 bug Something isn't working 😅 effort: medium Should be doable in <12h
Milestone

Comments

@timobrembeck
Copy link
Member

timobrembeck commented Feb 15, 2022

Describe the Bug

At the moment, we keep the file path identical when we're replacing media files.
This has the disadvantage that the app needs to download the files again to check whether it has changed. It would be more efficient if we treat filenames more like unique ids of files and change them in the content when they are replaced.

Steps to Reproduce

  1. Insert media file into content of page
  2. Replace file in media library
  3. Refresh preview of page
  4. Even though the timestamp in the media library itself is updated, the content remains the same.

Expected Behavior

When a media file is replaced in the media library, the new image should also be shown in the apps.

We can use the Link model of the linkcheck app to get the mapping between media files and page translations which contain the file. Then, we can update the url in the content.

Actual Behavior

Since the file path doesn't change, it is cached at the moment

Additional Information

Related issues:

@timobrembeck timobrembeck added 🐛 bug Something isn't working ❓ question Further information is requested ⁉️ prio: low Not urgent, can be resolved in the distant future. labels Feb 15, 2022
@timobrembeck timobrembeck added this to the Version 1.2 milestone Feb 15, 2022
@ulliholtgrave ulliholtgrave added the 😅 effort: medium Should be doable in <12h label May 16, 2022
@svenseeberg svenseeberg modified the milestones: 22Q2, 22Q3 Jun 7, 2022
@svenseeberg svenseeberg added ❗ prio: medium Should be scheduled in the forseeable future. and removed ⁉️ prio: low Not urgent, can be resolved in the distant future. labels Jun 7, 2022
@timobrembeck timobrembeck modified the milestones: 22Q3, 22Q4 Oct 2, 2022
@osmers
Copy link

osmers commented Oct 10, 2022

Referring here to issue #1742 - it would still be helpful to have an updated and an original date, so that I know that the file has been updated - it if is not too much hassle

@timobrembeck timobrembeck modified the milestones: 22Q4, 23Q1 Dec 12, 2022
@timobrembeck timobrembeck changed the title Replaced media files in content are cached Update file paths in content when media file is replaced Jan 17, 2023
@steffenkleinle
Copy link
Member

steffenkleinle commented Jan 17, 2023

Could you notify the app team in this issue once this is done please? Thanks :)

@sarahsporck
Copy link
Contributor

I'm wondering what makes most sense here. Currently files are stored like this: {subdirectory}/{strftime('%Y/%m')}/{filename} and the new (replacing) file only modifies certain attributes of the media_file.

  • upload the new file at e.g. {subdirectory}/{int(time())}/{filename}
  • delete the old file at the other path

Am I missing something? :)

@timobrembeck
Copy link
Member Author

timobrembeck commented Oct 23, 2023

Hmm, I guess I would stick with the year/month naming scheme instead of int(time()), so maybe it would already suffice to just remove the following lines?

# If the instance already exists in the database, make sure the upload path doesn't change
if instance.id:
original_instance = MediaFile.objects.get(id=instance.id)
if original_instance.file:
logger.debug(
"%r already exists in the database, keeping original upload path: %r",
instance,
original_instance.file.name,
)
return original_instance.file.name

And I think Django does not automatically delete the old file, yes so we'd have to do that manually.
And after that, we need to do a search & replace of the paths in the whole content to make sure we don't get a bunch of 404 errors on the old file path.

@sarahsporck
Copy link
Contributor

Thank you for the hint. Was a little bit stuck here.

@sarahsporck sarahsporck removed their assignment Nov 25, 2023
@MizukiTemma MizukiTemma removed the ❓ question Further information is requested label Dec 5, 2023
@MizukiTemma MizukiTemma modified the milestones: 23Q4, 24Q1 Dec 5, 2023
@david-venhoff david-venhoff self-assigned this Mar 11, 2024
@JoeyStk JoeyStk added ‼️ prio: high Needs to be resolved ASAP. and removed ❗ prio: medium Should be scheduled in the forseeable future. labels Apr 3, 2024
@JoeyStk JoeyStk modified the milestones: 24Q1, 24Q2 Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
‼️ prio: high Needs to be resolved ASAP. 🐛 bug Something isn't working 😅 effort: medium Should be doable in <12h
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants