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

Fix CloudinaryImages multi image upload functionality. #4957

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix CloudinaryImages multi image upload functionality. #4957

wants to merge 2 commits into from

Conversation

1337cookie
Copy link
Contributor

@1337cookie 1337cookie commented Aug 21, 2019

Description of changes

Change lodash flatten to flattenDeep. The lodash flatten function Flattens `array` a single level deep. which is inadequate for more than 2 image uploads as outlined in this comment. The lodash function flattenDeep Recursively flattens a nested array.

Related issues (if any)

Cloudinary: multiple images upload not working #4857

This may or may not be fixed in PR https://github.com/keystonejs/keystone/pull/4774 which is unlikely to be merged due to its size.

Testing

npm run test-all ran the same before and after changes we're made on Windows 10 and Ubuntu 18.04 (running in WSL).
I don't foresee any browser issues associated with this.
This error is thrown during tests which I have seen elsewhere and don't believe it is affected by this PR:

  1 failing
  1) FieldType: Markdown: Filter "before all" hook:
     Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.
  • List browser version(s) any admin UI changes were tested in:
    Windows Chrome Version 76.0.3809.100 (Official Build) (64-bit)
  • Please confirm you've added (or verified) test coverage for this change.
  • Please confirm npm run test-all ran successfully.

@1337cookie 1337cookie changed the title Change lodash flatten to flattenDeep (recursive) Fix CloudinaryImages multi image upload functionality. Aug 21, 2019
Copy link

@ankeris ankeris left a comment

Choose a reason for hiding this comment

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

This fixes the issue. Tested locally and all (multi) selected images work when uploading

Copy link
Member

@gautamsi gautamsi left a comment

Choose a reason for hiding this comment

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

LGTM

@ankeris
Copy link

ankeris commented Jun 11, 2020

Can we still expect a minor/patch version release with this fix?

stefy added a commit to stefy/keystone-classic that referenced this pull request Jan 23, 2021
Fix CloudinaryImages multi image upload functionality. keystonejs#4957
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants