Repair Cloudinary Select and extend upload functionality v2

[chasms]

Rebase of #4633

Description of changes

• Restored select functionality of CloudinaryImageType input field https://github.com/keystonejs/keystone/issues/4099

• Restored selectPrefix option functionality of CloudinaryImageType schema type so that select options are scoped to the correct path in Cloudinary

• Added support for additional filetypes in Cloudinary, including video

• Updated behavior of CloudinaryImageType with filenameAsPublicID option to use the filename and to throw a semantic error when filename already exists as related to discussion here: https://github.com/keystonejs/keystone/pull/925

• Set default behavior of CloudinaryImageType to use the Cloudinary use_filename: true convention - by which the publicID is set to the filename + a short uniqueID on upload to prevent collision - and use retry for whenExists

• Fixed issue where CloudinaryImageType's getExists was checking filename without first sanitizing and removing extension, causing false negatives

• Added new uploadOptions for Cloudinary so that upload presets, eager transformations, and other cloudinary upload options can be set per field on the schema all in an object that is passed to the upload request (so that all options are accommodated): https://github.com/keystonejs/keystone/issues/832, https://github.com/keystonejs/keystone/issues/4459, https://github.com/keystonejs/keystone/issues/2360, https://github.com/keystonejs/keystone/issues/2260, https://cloudinary.com/blog/centralized_control_for_image_upload_image_size_format_thumbnail_generation_tagging_and_more

• Added detail error messages for "Field Error" errors in EditForm AlertMessages, e.g. from Cloudinary upload responses when whenExists: 'error': https://github.com/keystonejs/keystone-storage-adapter-s3/issues/9

• Added detail error messages for custom pre('save' and pre('validate' hooks by adding error.detail into the res.apiError call of the Update route: https://github.com/keystonejs/keystone/issues/3348 Inside of the hooks, errors should be compiled like this to activate this functionality:

Post.schema.pre('validate', async function (next) {
    const error = []

    // on validations, push into the errors array
    // with an object with 'fieldLabel' and 'message' fields
    if (!this.title) {
        errors.push({ fieldLabel: 'title', message: 'No title set' });
    }

    // at end of validation, call next
    // with the compiled errors as the detail
    if (errors.length) {
        const error = new Error('field errors');
        error.detail = errors;
        next(error);
    } else {
        next();
    }
});

• Added defensive code to prevent breaking UI when cloudinaryImageSummary attempts to render empty image in table cell

• Added textArea character counts on UI per spec in #126

Related issues (if any)

• Clean up old getRequestHandler methods https://github.com/keystonejs/keystone/issues/3433
• Add option to display character count under Text fields #140

Testing

• [ ] Please confirm npm run test-all ran successfully.

[barbarosso]

I would like to see this one get merged into master. Are there any loose ends here or can we help somewhere to get this one released?

Kind regards Pascal

[stennie]

@barbarosso This PR is an updated version of #4633 but still has the same issues in terms of being a monolithic change set that is difficult to review & test. There don't appear to be any documentation or test changes, which seems surprising for 48 commits and 19 files changed. The PR also doesn't mention what testing has been done (for example, browser support).

To make this more easily reviewable I would still be inclined to cherrypick and group related fixes, add tests and documentation, and submit several individual PRs. For example, the "Flash messages in admin UI list view" could be a distinct and reviewable changeset that should likely be done first (since the Cloudinary changes make use of this).

I would like to see this one get merged into master.

I'd also like to see the changes reviewed and merged, but there's currently a lot of prep work required with a PR of this size and scope. Keystone contributors are volunteering their time and also inherit the support and maintenance of code and documentation once changes are merged into master.

Some help breaking up the monolith into easily reviewable PRs (including tests & docs) would definitely move this along faster. For some tips on great pull requests, see: https://www.atlassian.com/blog/git/written-unwritten-guide-pull-requests.

Regards, Stennie

[chasms]

@barbarosso @stennie Yep, this is something I definitely want to split up and make easier for review, just haven't had any time lately. Any help would be much appreciated (and I'm glad to coordinate if you send me a message)!

[barbarosso]

@chasms @stennie I think i can help some hours next week, but if some preperation could be done to make it easier to help out.

[RecuencoJones]

@chasms, I just checked your branch and it looks great!

I did find one minor issue though, when selecting from the dropdown it gets cut by the modal instead of overflowing:

I think it can be fixed by changing overflow-y: initial in the modal element:

[laurenskling]

Hey @chasms , we are entering maintenance mode for keystone 4 ( see https://github.com/keystonejs/keystone/issues/4913). I would love to get some of your work on Cloudinary into Keystone. Would you be open to helping us fix the issues you've raised here, but as seperate small PR's? This would make it more controllable and testable. We can work on it together to create a step by step upgrade plan to get your fixes into keystone.

[chasms]

@laurenskling I need to find some time for this. First thing I imagine is to rebase and smoke test, make sure it is still working with the latest. I've lost a lot of context since I last worked on this, and it is a bit monolithic but a lot of pieces are interdependent. Will likely take some effort to split up. I'll keep you posted on my ability to find some time for this.

[laurenskling]

Just a thought; but I think it might be the best plan to take this PR as a reference in what to do. Take care of each bullet point, in good order, one by one. By creating new PR's. There hasn't changed much in the codebase since this PR. I can pretty much safely say: nothing in CloudinaryImage.

This is a big thing for me, as one of the new maintainers, to get CloudinaryImage back in shape. I'm pretty sure most users of Keystone, like me, use this type alot. Would be great to work together to get stuff fixed. You've got my promise I will put in the effort to help you.

[chasms]

@laurenskling sounds good. Do you have a demo project with resources all set up for testing the package live with link? I'll need to get a dev setup going again.

[laurenskling]

I don't have anything like that ready to go at the moment, no. :/