Skip to content

Conversation

DhruvBasan
Copy link
Contributor

@DhruvBasan DhruvBasan commented Sep 29, 2023

Title

The folder-tree are not visible and getting error “We can't upload the file to the current folder right now. Please try another folder.” when we try to upload image

Description (*)

There is an issue encountered when attempting to upload an image to a folder within the WYSIWYG editor on a product page, specifically within the short description in content section. The folders are not visible, and as a result, we cannot upload the image and we receive an error message stating, 'We can't upload the file to the current folder right now. Please try another folder.' This problem only occurs when you expand the 'Image and Video' tab first. And getting the error in console as shown in below image

image

Manual testing scenarios (*)

1: Access the Magento admin panel and navigate to 'Catalog' -> 'Products.' and Select any product.

2: in the product page, first, expand the 'Image and Videos' tab, as shown in the image below.

image

3: Next, expand the 'Content' tab and in 'Short Description' section click on the 'Insert Image' button and select the 'Source' button in the WYSIWYG editor, as indicated in the image below

image

4: after clicking the 'Source' button, you may notice that the folders are not displayed, and as a result, you are unable to upload images within the 'Short Description.' as you can see in below image

image

**Expected result **

After expanding image and videos tab first we should be able to see folder option and can insert image in short description

**Actual result **

After expanding image and videos tab first we can not see folder option and can not insert image in short-description

Aditional information

In this issue, when we expand the 'Image and Videos' tab and then attempt to upload an image within the 'Short Description' content section, it triggers the 'tmp.init(el,options);' in the lib/web/jquery/jstree/jquery.jstree.js file at line 123, as illustrated in the image below.

image

After the 'tmp.init(el, options)' function is triggered in the 'jquery.jstree.js' file, it call the init function in the 'tree-suggest.js' file. Please note that it call init function in the 'tree-suggest.js' file only when we initially expand the 'Image and Videos' tab first.

In the lib/web/mage/backend/tree-suggest.js file, it calls the ‘init’ function at line 58 however, an issue arises because the ‘init’ function does not receive the element. Consequently, ‘this.get_container’ becomes undefined

This issue likely plays a significant role in the failure to display folders in the WYSIWYG editor when attempting to upload an image.the element contains the data of folder-tree so if it not assign correct we can not see folder tree when we try to insert image using WYSIWYG editor in short-description

image
In the image provided above, you can see on line 77 and also on line 102, there is a call to the '_get_node' method. However, there isn't a method with this name in the file or jquery.jstree.js file. Instead, the correct method name is 'get_node.' Therefore, we need to make this correction to ensure the proper function is called.

Solution

To resolve the issue, we need to make changes in the 'lib/web/mage/backend/tree-suggest.js' file. Specifically, we should pass the 'element' in the 'init' function at line 58. Afterward, we should call the 'init' function before 'get_container' and pass the 'data' (consisting of 'element' and 'option') to ensure that the 'init' function receives the required element. This modification ensures that the 'container' variable is not undefined.

Additionally, we should replace instances of '_get_node' with 'get_node' on lines 77 and 102 to ensure that the correct method is called.

Once these modifications are implemented, the issue should be successfully resolved.you can see changes in below image

image

Questions or comments

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
  • All automated tests passed successfully (all builds are green)

Resolved issues:

  1. resolves [Issue] file upload issue in WYSIWYG editor on product page #38026: file upload issue in WYSIWYG editor on product page

@m2-assistant
Copy link

m2-assistant bot commented Sep 29, 2023

Hi @DhruvBasan. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.

Add the comment under your pull request to deploy test or vanilla Magento instance:
  • @magento give me test instance - deploy test instance based on PR changes
  • @magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

  • @magento run all tests - run or re-run all required tests against the PR changes
  • @magento run <test-build(s)> - run or re-run specific test build(s)
    For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:
  1. Database Compare
  2. Functional Tests CE
  3. Functional Tests EE
  4. Functional Tests B2B
  5. Integration Tests
  6. Magento Health Index
  7. Sample Data Tests CE
  8. Sample Data Tests EE
  9. Sample Data Tests B2B
  10. Static Tests
  11. Unit Tests
  12. WebAPI Tests
  13. Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.


For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

@DhruvBasan
Copy link
Contributor Author

@magento run Unit Tests

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

@engcom-Bravo
Copy link
Contributor

@magento create issue

@engcom-Bravo engcom-Bravo added the Priority: P2 A defect with this priority could have functionality issues which are not to expectations. label Sep 29, 2023
@DhruvBasan
Copy link
Contributor Author

@magento run all tests

Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

@sinhaparul sinhaparul added the Project: Community Picked PRs upvoted by the community label Jan 25, 2024
Copy link
Contributor

@andrewbess andrewbess left a comment

Choose a reason for hiding this comment

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

Hello @DhruvBasan
Thank you for your contribution
Could you please check my suggestions and fix PR
Thank you in advance.

Comment on lines 58 to 59
init: function (el,options) {
init.call(this,el,options);
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add spaces between params

Suggested change
init: function (el,options) {
init.call(this,el,options);
init: function (el, options) {
init.call(this, el, options);

Perhaps JS Coding Style requires it
https://javascript.info/coding-style

Copy link
Contributor Author

@DhruvBasan DhruvBasan Jan 29, 2024

Choose a reason for hiding this comment

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

Hello @andrewbess , I have implemented the suggestions you provided on my pull request

@engcom-Hotel
Copy link
Contributor

@magento run all tests

Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

@engcom-Hotel
Copy link
Contributor

@magento run Functional Tests B2B, Functional Tests EE, Integration Tests, WebAPI Tests

Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

Copy link
Contributor

@engcom-Hotel engcom-Hotel left a comment

Choose a reason for hiding this comment

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

Hello @DhruvBasan,

Thanks for the collaboration!

The code changes seems good to us. But in accordance of DOD, please cover the changes with some automated test.

Thanks

@engcom-Echo
Copy link
Contributor

WebAPI Tests failure is known failure and not related to PR changes.
Functional Tests CE and Functional Tests B2B failure are different on last two run on same commit.
Functional Tests CE
Run1
Screenshot 2024-02-08 at 4 08 53 PM

Run2
Screenshot 2024-02-08 at 4 09 11 PM

Functional Tests B2B
Run1
Screenshot 2024-02-08 at 4 08 59 PM

Run2
Screenshot 2024-02-08 at 4 09 33 PM

Hence Moving it to Merge In Progress

@DhruvBasan
Copy link
Contributor Author

@engcom-Echo your changes in the test file (app/code/Magento/Cms/Test/Mftf/Section/TinyMCESection/MediaGallerySection.xml) are conflicting with the developer branch. Could you please review and resolve the conflict?

@engcom-Echo
Copy link
Contributor

@magento run all tests

@engcom-Echo
Copy link
Contributor

@magento run Static Tests

@engcom-Echo
Copy link
Contributor

@magento run Static Tests,Integration Tests,Functional Tests CE,Functional Tests B2B

@engcom-Echo
Copy link
Contributor

@magento run all tests

@engcom-Echo
Copy link
Contributor

@magento run all tests

@engcom-Echo
Copy link
Contributor

@magento run all tests

@engcom-Echo
Copy link
Contributor

@magento run WebAPI Tests,Functional Tests B2B

@engcom-Echo
Copy link
Contributor

@magento run all tests

@engcom-Echo
Copy link
Contributor

@magento run Functional Tests EE,Functional Tests B2B,Integration Tests,WebAPI Tests

@engcom-Echo
Copy link
Contributor

@magento run Functional Tests EE ,Integration Tests

@engcom-Echo
Copy link
Contributor

engcom-Echo commented Mar 5, 2024

**Integration Tests ** failure are known flaky failure.
Screenshot 2024-03-21 at 12 22 17 PM

Functional Tests B2B,Functional Tests EE failures are different on last two run on same commit.
Functional Tests B2B
Run1
Screenshot 2024-03-05 at 12 44 11 PM

Run2
Screenshot 2024-03-05 at 12 44 17 PM

Functional Tests EE
Run1
38025-ee

Run2
Screenshot 2024-03-05 at 2 25 08 PM

Hence Moving it to merge in progress

@hostep
Copy link
Contributor

hostep commented May 16, 2024

Looks like this was merged in 2.4-develop recently in c6bc830

However, PR isn't marked as merged, probably because of the last 2 merge commits part of this PR.
So I guess this PR can be closed?

@engcom-Echo
Copy link
Contributor

Closing this PR as it is merged to 2.4-develop. Commit link can be found here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Award: MFTF test coverage Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Project: Community Picked PRs upvoted by the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Issue] file upload issue in WYSIWYG editor on product page
8 participants