-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Cloudinary usability improvements #15091
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
Changes from all commits
b049151
531d3d9
13b8822
b474087
d9a54c5
abe2c20
c482de6
c43afd8
45ca93f
57a81ee
7b01566
cd1d313
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,4 +8,4 @@ export default { | |
| console.log(Object.keys(this.$auth)); | ||
| }, | ||
| }, | ||
| }; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,4 +8,4 @@ export default { | |
| console.log(Object.keys(this.$auth)); | ||
| }, | ||
| }, | ||
| }; | ||
| }; | ||
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,65 @@ | ||||||||||||||||||||||||||||||||||
| import { ConfigurationError } from "@pipedream/platform"; | ||||||||||||||||||||||||||||||||||
| import cloudinary from "../../cloudinary.app.mjs"; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export default { | ||||||||||||||||||||||||||||||||||
| key: "cloudinary-transform-resource", | ||||||||||||||||||||||||||||||||||
| name: "Transform Resource", | ||||||||||||||||||||||||||||||||||
| description: "Transform an image, video or audio asset on-the-fly with several options. [See the documentation](https://cloudinary.com/documentation/video_manipulation_and_delivery)", | ||||||||||||||||||||||||||||||||||
| version: "0.0.1", | ||||||||||||||||||||||||||||||||||
| type: "action", | ||||||||||||||||||||||||||||||||||
| props: { | ||||||||||||||||||||||||||||||||||
| cloudinary, | ||||||||||||||||||||||||||||||||||
| assetId: { | ||||||||||||||||||||||||||||||||||
| propDefinition: [ | ||||||||||||||||||||||||||||||||||
| cloudinary, | ||||||||||||||||||||||||||||||||||
| "assetId", | ||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
| info: { | ||||||||||||||||||||||||||||||||||
|
Check warning on line 18 in components/cloudinary/actions/transform-resource/transform-resource.mjs
|
||||||||||||||||||||||||||||||||||
| type: "alert", | ||||||||||||||||||||||||||||||||||
| alertType: "info", | ||||||||||||||||||||||||||||||||||
| content: `You can either select a pre-configured transformation or pass a transformation string. Both can be managed in the [Cloudinary Transformation Builder](https://tx.cloudinary.com/). | ||||||||||||||||||||||||||||||||||
| \\ | ||||||||||||||||||||||||||||||||||
| If both are specified, the transformation string will be ignored.`, | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+18
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Ensure the Static analysis indicates that component props of type Below is a suggested fix: info: {
type: "alert",
alertType: "info",
+ label: "Information",
+ description: "Guidance on selecting a named transformation or providing a transformation string.",
content: `You can either select a pre-configured transformation...
If both are specified, the transformation string will be ignored.`,
},📝 Committable suggestion
Suggested change
🧰 Tools🪛 GitHub Check: Lint Code Base[warning] 18-18: [warning] 18-18: |
||||||||||||||||||||||||||||||||||
| namedTransformation: { | ||||||||||||||||||||||||||||||||||
| propDefinition: [ | ||||||||||||||||||||||||||||||||||
| cloudinary, | ||||||||||||||||||||||||||||||||||
| "namedTransformation", | ||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
| transformationString: { | ||||||||||||||||||||||||||||||||||
| propDefinition: [ | ||||||||||||||||||||||||||||||||||
| cloudinary, | ||||||||||||||||||||||||||||||||||
| "transformationString", | ||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
| async run({ $ }) { | ||||||||||||||||||||||||||||||||||
| const { | ||||||||||||||||||||||||||||||||||
| cloudinary, assetId, namedTransformation, transformationString, | ||||||||||||||||||||||||||||||||||
| } = this; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if (!namedTransformation && !transformationString) { | ||||||||||||||||||||||||||||||||||
| throw new ConfigurationError("Either `Named Transformation` or `Transformation String` are required"); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||
| const response = await cloudinary.transformAsset(assetId, namedTransformation | ||||||||||||||||||||||||||||||||||
| ? { | ||||||||||||||||||||||||||||||||||
| transformation: namedTransformation, | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| : { | ||||||||||||||||||||||||||||||||||
| raw_transformation: transformationString, | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if (response) { | ||||||||||||||||||||||||||||||||||
| $.export("$summary", "Successfully transformed resource"); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| return response; | ||||||||||||||||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||||||||||||||||
| throw new Error(`Cloudinary error response: ${err.error?.message ?? JSON.stringify(err)}`); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
🛠️ Refactor suggestion
Add metadata property to options object
The review comment is correct. The
metadataproperty is defined in the component's props but not included in the options object passed to the Cloudinary API call. Here are the specific locations that need attention:components/cloudinary/actions/get-resources/get-resources.mjs, add themetadataproperty to the options object in therunmethod:🔗 Analysis chain
Verify metadata property integration with Cloudinary API
The new metadata property is well-documented, but it needs to be included in the options object passed to the Cloudinary API.
Add the metadata property to the options object in the run method:
const options = { resource_type: this.resourceType, type: this.type, prefix: this.prefix, tags: this.tags, context: this.context, moderation: this.moderation, + metadata: this.metadata, };Let me verify if the metadata property is properly integrated with the Cloudinary API by checking the implementation.
Let me fix the regex and try again.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
Length of output: 190
Script:
Length of output: 674