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

Add support for Base Folder #7

Closed
thomasvantuycom opened this issue Dec 7, 2023 · 6 comments
Closed

Add support for Base Folder #7

thomasvantuycom opened this issue Dec 7, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@thomasvantuycom
Copy link
Owner

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 #5 (comment)

@thomasvantuycom thomasvantuycom added the enhancement New feature or request label Dec 7, 2023
@thomasvantuycom
Copy link
Owner Author

This is now available in version 1.5.0.

@valboivin
Copy link

@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
Copy link
Owner Author

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
Copy link

valboivin commented Jan 3, 2024

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
Copy link
Owner Author

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

@valboivin
Copy link

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. 😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants