search

metakgp / metakgp-wiki

Dockerized source for the metakgp wiki.
https://wiki.metakgp.org
GNU General Public License v3.0
23 stars 20 forks source link

Change the way `graphicsmagick` is used for creating thumbnails; include SimpleBatchUpload extension #48

Closed icyflame closed 5 years ago

icyflame commented 5 years ago

I have tested this now. Uploading an image immediately generates it's thumbnail.

The SimpleBatchUpload extension will work well for us. I uploaded images that are missing (downloaded from the old snapshot we had) and they automatically appeared on pages that they were missing from.

icyflame commented 5 years ago

More testing required. I am closing this for now. I will re-open it once I have tested it thoroughly locally.

amrav commented 5 years ago

Awesome, thanks for doing this :D

Why graphicmagick instead of imagemagick?

I noticed you added config for SVG and PDF, did you test those two?

Would be great to have a test for image uploads (given how error prone this is - might catch regressions between versions of various libraries etc), and shouldn't be too hard to add (you could use the api through curl), but not a blocker :)

icyflame commented 5 years ago

SVG uploading is not allowed, so, I have removed that from the config. I have tested PDF and image uploading.

I didn't realize the difference between GraphicsMagick and ImageMagick when I used it. I saw that the existing converter in the config was GraphicsMagick and just stuck with it.

I have read up on the two now. GM was forked from ImageMagick in 2002 and has been an independent project since. It seems to be an active project, with frequent releases. I don't think we can go wrong with using either of these two. I think we should stick with gm since the installation (through apt) for it is already written in php/Dockerfile.

I take your point on image testing. I was actually going to write this yesterday itself, and I opened the tests/ folder, but I need to find out a bit more on how tests really work and how to run/write them. So, I will do that and add them in a later pull request.