Add support for Base Folder

thomasvantuycom commented 10 months ago

I still have one minor problem with the generated URL for the transformation to work.

In our Cloudinary setup, we do use the fixed folders structure. We have base folders for every environments we are using. I know there's a way for the Upload API to specify a folder name in the upload options, but I don't see any for the transformation.

Could I suggest some more enhancements to the plugin to support base folder? I've modified the code on my side to make it work. Here's what I suggest:

Adding a field in the plugin to specifiy the base folder name and make it a property of the CloudinaryFs class
```
    public string $baseFolder = '';
```

In the getTransformUrl function, prepend the publicId with this base folder value

    $hasBaseFolder = !(empty($fs->baseFolder));

    $publicId = $asset->getUrl();
    if ($isCloudinaryFs) {
        $publicId = $hasDynamicFolders ? basename($asset->getPath()) : $asset->getPath();
    }
    if($hasBaseFolder) {
        $publicId = '/'. $fs->baseFolder . '/' . $publicId;
    }

Let me know what you think!

Originally posted by @valboivin in https://github.com/thomasvantuycom/craft-cloudinary/issues/5#issuecomment-1845736107

thomasvantuycom commented 9 months ago

This is now available in version 1.5.0.

valboivin commented 9 months ago

@thomasvantuycom, thank you so much! 👍

It seems like there's just a tiny error in CloudinaryFs.php. When appending the base folder to the path, it doesn't parse the environment variable so it gets hardcoded in the asset url. Basically, we could juste replace line 57 with:

$baseFolder = Craft::parseEnv($this->baseFolder);

With this change, everything seems to work fine. I can preview, edit, delete and fetch any asset. 😃

thomasvantuycom commented 9 months ago

Hi Valerie,

I overlooked parsing the environment variable, but in a different location than you indicated. I'm unsure why your suggested fix is effective. The correction should involve modifying line 30 in CloudinaryTransformer.php to

$publicId = Craft::parseEnv($fs->baseFolder) . '/' . $publicId;

Can you verify if it functions as expected and assure me that I'm hallucinating?

valboivin commented 9 months ago

Hi @thomasvantuycom ,

you're right I had changed it in both places for everything to function as expected! I just forgot about this one, oops. It definitely has to be parsed in the CloudinaryTransformer.php as you mentioned.

I'm just not sure why we wouldn't want to parse it in the query of the CloudinaryFs.php.

thomasvantuycom commented 9 months ago

We do parse it there, only a couple more lines down.

valboivin commented 9 months ago

Oh I see! I thought that was the place I had changed it. So basically, it is just missing in the CloudinaryTransformer.php. Sorry for the confusion, I think I didn't take enough coffee yesterday. 😆

thomasvantuycom / craft-cloudinary

Add support for Base Folder #7