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

remove cloudinary provider module #1542

Merged
merged 19 commits into from
Oct 31, 2024

Conversation

dbauszus-glx
Copy link
Member

@dbauszus-glx dbauszus-glx commented Oct 8, 2024

The cloudinary provider module which requires the image/document payload to be passed through the xyz host has been removed.

The signer module has now been reviewed and documented.

The cloduinary signer module has now been reviewed and documented.

The module will not fail silently but return an error message.

The cloudinary entry is now using the cloudinary signer module.

The cloudinary entry is now reviewed and documented.

The cloudinary entry requires a cloudinary account. However, no third party module is required to sign requests, hence the cloudinary signer is included in the XYZ core and the cloudinary entry method is included in the mapp location entries.

@dbauszus-glx dbauszus-glx self-assigned this Oct 8, 2024
@dbauszus-glx dbauszus-glx added Code Issues related to the code structure and performance. Plugin Refers to an issue in a plugin or a feature which should be resolved in a plugin. Documentation Adding or updating documentation. labels Oct 8, 2024
@dbauszus-glx dbauszus-glx linked an issue Oct 8, 2024 that may be closed by this pull request
express.js Dismissed Show dismissed Hide dismissed
@dbauszus-glx dbauszus-glx marked this pull request as ready for review October 16, 2024 16:24
Copy link
Member

@cityremade cityremade left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as expected.
I've replaced alerts and confirms with ui elements plus more human wording on removal confirm;

Copy link
Contributor

@RobAndrewHurst RobAndrewHurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

when removing a document that uses cloudinary the alert says I am going to remove an image which doesn't align with the action.

We should change this to something more fitting for documents

Copy link
Contributor

@RobAndrewHurst RobAndrewHurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

All of these dictionary entries except the remove_item_confirm are not used anywhere in the core, and should be removed.

@RobAndrewHurst
Copy link
Contributor

Works as expected. I've replaced alerts and confirms with ui elements plus more human wording on removal confirm;

This can't work with both images and documents.

Copy link
Contributor

@RobAndrewHurst RobAndrewHurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have have replaced the remove_item_confrim entry on the dictionary with two items. One for images and the other for documents.

They are then passed to the trash method as params.

@RobAndrewHurst RobAndrewHurst self-requested a review October 31, 2024 10:30
@RobAndrewHurst RobAndrewHurst changed the base branch from main to minor October 31, 2024 10:52
Copy link
Contributor

@AlexanderGeere AlexanderGeere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Writing tests for the module

Copy link

Copy link
Contributor

@RobAndrewHurst RobAndrewHurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy with this!

@RobAndrewHurst RobAndrewHurst merged commit ae2edf9 into GEOLYTIX:minor Oct 31, 2024
2 checks passed
@dbauszus-glx dbauszus-glx deleted the cloudinary-entries-sign branch November 27, 2024 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Issues related to the code structure and performance. Documentation Adding or updating documentation. Plugin Refers to an issue in a plugin or a feature which should be resolved in a plugin.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cloudinary provider and sign
4 participants