Project

General

Profile

Actions

Bug #21483

closed

CV cover/profile image URL generation not compatible with S3-Uploads

Added by Boone Gorges 2 months ago. Updated 8 days ago.

Status:
Resolved
Priority name:
Normal
Assignee:
Category name:
CV
Target version:
Start date:
2024-11-13
Due date:
% Done:

0%

Estimated time:
Deployment actions:

Description

It appears that CV cover/profile images are not fully compatible with S3-Uploads. The uploaded images are sent to S3, but the image URLs on the front end don't point to S3. I think this is because of Media\get_profile_image_base_url() and Media\get_cover_image_base_url(), which use home_url() rather than wp_upload_url() and thus aren't filtered by the S3-Uploads plugin.

I thought about making the necessary change myself, but I was worried I'd break something :-D Jeremy, could you please have a look, and also think about whether there are other related interfaces that might be affected (ie default cover images)?


Related issues

Related to CUNY Academic Commons - Feature #21383: Offload media using S3-UploadsNewBoone Gorges2024-11-01

Actions
Actions #1

Updated by Boone Gorges 2 months ago

Actions #2

Updated by Boone Gorges 9 days ago

  • Status changed from New to Resolved
Actions #3

Updated by Jeremy Felt 9 days ago

Hey Boone - I just missed you on this one, sorry about that.

I've got a fix/cv-s3-uploads branch up that should be a bit more comprehensive:

1. The profile image issue is an easy switch to wp_upload_dir()
2. The cover images were (face palm) being stored very badly with full URLs in the database. I've refactored this so that only the image name is stored and the full URL is built dynamically.

I'm not sure if that's useful in 2.5.x yet or in the reclaim-migration branch, happy to merge it into either or for you to!

Actions #4

Updated by Boone Gorges 9 days ago

Thanks, Jeremy! This approach looks good. Feel free to roll back what I put in the reclaim-migration branch and merge it there.

Actions #5

Updated by Colin McDonald 9 days ago

Thanks from me too, Jeremy. If you could update this ticket when the new code is available in the reclaim-migration branch to test, the testing team would like to have a look.

Actions #6

Updated by Boone Gorges 8 days ago

In the interest of time, I've merged the changes myself.

Actions

Also available in: Atom PDF