Basic site logos should hit the CDN

(Michael Brown) #1

When we have a CDN defined, we should be leveraging that for various image assets (e.g. logo url, logo small url) that are relative URLs

e.g.: logo_url='/uploads/default/original/1X/5b5cc13aa92459bb084ca1485bca80c480bfa6f0.gif'

(Régis Hanol) #2

Where do you see that exactly?

(Michael Brown) #3

sjcthree for example - the site logos were uploaded during the wizard - I haven’t changed them.

(Régis Hanol) #6

@nbianca can you make sure the images in the site settings use the CDN whenever they’re pointing at local images?

(Bianca) #8

I tried reproducing this with latest-version and v1.9.0 (had to cherry-pick a6c589d8). My CDN was provided by a local Minio instance.

The code also looks fine as it uses the same logic for other uploads.

@supermathie, have you seen it reproducing recently?

(Michael Brown) #9

I haven’t set up any new sites recently, but I’ll re-run the wizard on a site and try it.

(ginger man) #10

I just tried this with latest discourse version on a new site.

image uploads for logos only use the s3 urls instead of given s3_cdn url. (but S3 cdn url is correctly picked up in image uploads within a post)

Also, discourse used to have a text box under the logo images where we can fill a custom image link. That was quite useful. Even in this case, we could have manually updated the CDN url if there is a text box.

#11

See Confused by the new logo UI in site settings

(ginger man) #12

@hawk thanks for the note. I am part of the conversations there.

I am actually looking for a fix / way to get the s3_cdn url in logos. Having a text box, is just a temporary fix.

(Bianca) #13

Thank you for your report, @gingerman. I had another look and found the culprit.

I submitted a pull request to fix the issue and the changes will be merged soon.

(Bianca) closed #14
Upgrade to Beta8 deletes all icons