Provide pathId param on browser.js #28219

Nazar65 · 2020-05-14T08:12:09Z

Description (*)

Fixed Issues (if relevant)

Fixes magento/adobe-stock-integration#1335: Requested default folder is not selected on opening media gallery (current_tree_path parameter)

Manual testing scenarios (*)

Open media gallery from the category image field
Open media gallery from the WYSIWYG

Expected result (*)

catalog/category directory is selected by default
WYSIWYG directory is selected by default

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)
All automated tests passed successfully (all builds are green)

m2-assistant · 2020-05-14T08:12:12Z

Hi @Nazar65. Thank you for your contribution
Here is some useful tips 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 only 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). <test-build(s)> is a comma-separated list of build names. Allowed build names are Database Compare, Functional Tests CE, Functional Tests EE, Functional Tests B2B, Integration Tests, Magento Health Index, Sample Data Tests CE, Sample Data Tests EE, Sample Data Tests B2B, Static Tests, Unit Tests, WebAPI Tests.

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

Nazar65 · 2020-05-14T08:12:36Z

@magento run all tests

Nazar65 · 2020-05-14T08:18:14Z

@magento run all tests

Nazar65 · 2020-05-14T09:18:07Z

@magento run all tests

gabrieldagama

Hi @Nazar65 thanks for your pull request, it looks good! Please follow below some comments. Thanks!

app/code/Magento/Cms/Model/Wysiwyg/Gallery/DefaultConfigProvider.php

lib/web/mage/adminhtml/browser.js

app/code/Magento/Cms/Test/Mftf/Test/AdminVerifyDefaultCurrentTreePathTest.xml

app/code/Magento/Ui/view/base/web/js/form/element/image-uploader.js

app/code/Magento/Cms/Test/Mftf/Section/TinyMCESection/MediaGallerySection.xml

app/code/Magento/Cms/Test/Mftf/Test/AdminVerifyDefaultCurrentTreePathTest.xml

Nazar65 · 2020-05-14T15:17:24Z

@magento run all tests

Nazar65 · 2020-05-14T18:32:57Z

@magento run all tests

Nazar65 · 2020-05-15T12:00:59Z

@magento run all tests

gabrieldagama

Hi @Nazar65, thanks for your changes! Please follow some small comments below.

gabrieldagama · 2020-05-18T12:58:36Z

app/code/Magento/Cms/view/adminhtml/web/js/folder-tree.js

                };

+            $(window).on('reload.MediaGallery', function () {
+                encodedPath = Base64.decode(window.MediabrowserUtility.pathId.replace(/--|,,/, '==')).split('/');


I believe this variable name should be decodedPath since it is decoding the pathId, correct?

gabrieldagama · 2020-05-18T13:01:53Z

app/code/Magento/Cms/view/adminhtml/web/js/folder-tree.js

+                tree.jstree('deselect_all');
+
+                if (encodedPath.length > 1) {
+                    path =  _parseCurrentPath(encodedPath);


The _parseCurrentPath doesn't accept any parameter, I believe we can remove the encodedPath from here.

gabrieldagama · 2020-05-18T14:32:28Z

app/code/Magento/Cms/view/adminhtml/web/js/folder-tree.js

+                        if (isLastElement) {
+                            paths[i] = window.MediabrowserUtility.pathId.replace(',,', '--');
+                        } else {
+                            paths[i] = Base64.encode(val).replace('==', '--');


I think the replace here isn't correct and may cause other problems, please follow below the way PHP is handling the encode, I believe we have to perform the same behavior on JS.

file: app/code/Magento/Cms/Helper/Wysiwyg/Images.php
method: idEncode
line: 304
return strtr(base64_encode($string), '+/=', ':_-');

Nazar65 · 2020-05-21T08:15:34Z

@magento run all tests

Nazar65 · 2020-05-21T09:34:51Z

@magento run all tests

Nazar65 · 2020-05-21T10:24:55Z

@magento run all tests

Nazar65 · 2020-05-21T12:46:36Z

@magento run WebAPI Tests

magento-engcom-team · 2020-05-21T14:36:43Z

Hi @gabrieldagama, thank you for the review.
ENGCOM-7573 has been created to process this Pull Request
✳️ @gabrieldagama, could you please add one of the following labels to the Pull Request?

Label	Description
`Auto-Tests: Covered`	All changes in Pull Request is covered by auto-tests
`Auto-Tests: Not Covered`	Changes in Pull Request requires coverage by auto-tests
`Auto-Tests: Not Required`	Changes in Pull Request does not require coverage by auto-tests

engcom-Bravo · 2020-05-22T09:14:12Z

✔️ QA Passed

Testing Scenario
Precondition:

Adobe Stock Integration branch cloned and required by the composer. (You can find the installation guide here)
Switch Adobe Stock to this if it is not merged yet

Steps:

Open Magento Admin
Go to Stores - Configuration - Advanced tab - System and select Enabled Yes for Enhanced Media Gallery. Save Config
Go to Catalog - Categories
(the Default Category is selected)
Expand Content
Click Select from Gallery button near the Category Image
Expected Result: Catalog/Category directory is selected
Close Media Gallery
Click Insert/edit image from Description editor
From the opened Insert/edit image window click Search button near the Source field
Expected Result: wysiwyg directory is selected

chalov-anton · 2020-05-25T11:52:02Z

Added test steps to already existing scenario on cucumber studio https://studio.cucumber.io/projects/131313/test-plan/folders/1054245/scenarios/4523889

m2-assistant · 2020-05-28T02:51:02Z

Hi @Nazar65, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

Nazar65 added 3 commits May 13, 2020 20:54

Fix isse with default selected tree_path

660eb75

Refactor file browser to use current_tree_path dynamically

f434c97

Cover changes with mftf tests

f905963

ghost added the Progress: pending review label May 14, 2020

magento-engcom-team added Area: Frontend Area: Lib/Frontend Component: Cms Component: Ui Release Line: 2.4 labels May 14, 2020

Nazar65 mentioned this pull request May 14, 2020

Respect (current_tree_path parameter) on opening media gallery magento/adobe-stock-integration#1350

Merged

Nazar65 added 2 commits May 14, 2020 11:16

Fix identation

bd6853f

Add missing property

d811f9a

Correct unit tests

b4fe70c

gabrieldagama changed the title ~~Fix current tree path preselected folder on MediaGallery, after reopen insatnce~~ Fix current tree path preselected folder on MediaGallery, after reopen instance May 14, 2020

gabrieldagama suggested changes May 14, 2020

View reviewed changes

ghost assigned gabrieldagama May 14, 2020

ghost added Progress: needs update and removed Progress: pending review labels May 14, 2020

Refactor per review comments

e8e654f

Improve preselected folders to parse pathId and open it recursively

65b11c3

gabrieldagama reviewed May 18, 2020

View reviewed changes

refactor per review comments

eab0af9

Add case to verify pathId

93f2144

Correct current_tree_path decode method

5e365a0

fix url mock for imag-uploader test

2340860

gabrieldagama approved these changes May 21, 2020

View reviewed changes

ghost added Progress: ready for testing and removed Progress: needs update labels May 21, 2020

magento-engcom-team removed the Component: Cms label May 21, 2020

gabrieldagama added the Auto-Tests: Covered All changes in Pull Request is covered by auto-tests label May 21, 2020

engcom-Bravo self-assigned this May 22, 2020

ghost added Progress: testing in progress Progress: extended testing Progress: accept and removed Progress: ready for testing Progress: testing in progress Progress: extended testing labels May 22, 2020

sivaschenko added this to the 2.4.0 milestone May 22, 2020

slavvka changed the base branch from 2.4-develop to 2.4.0-develop May 22, 2020 16:10

Nazar65 mentioned this pull request May 25, 2020

Replicate old media gallery behavior with current tree path magento/adobe-stock-integration#1385

Merged

chalov-anton added the QA: Added to Regression Scope Scenario was analysed and added to Regression Testing Scope label May 25, 2020

slavvka mentioned this pull request May 28, 2020

Widget Sort Orders. #1335

Closed

slavvka added a commit that referenced this pull request May 28, 2020

ENGCOM-7573: Provide pathId param on browser.js #28219

ce524ce

slavvka merged commit ec5e835 into magento:2.4.0-develop May 28, 2020

Provide pathId param on browser.js #28219

Provide pathId param on browser.js #28219

Uh oh!

Conversation

Nazar65 commented May 14, 2020 • edited by slavvka Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Expected result (*)

Questions or comments

Contribution checklist (*)

Uh oh!

m2-assistant bot commented May 14, 2020

Uh oh!

Nazar65 commented May 14, 2020

Uh oh!

Nazar65 commented May 14, 2020

Uh oh!

Nazar65 commented May 14, 2020

Uh oh!

gabrieldagama left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Nazar65 commented May 14, 2020

Uh oh!

Nazar65 commented May 14, 2020

Uh oh!

Nazar65 commented May 15, 2020

Uh oh!

gabrieldagama left a comment

Choose a reason for hiding this comment

Uh oh!

gabrieldagama May 18, 2020

Choose a reason for hiding this comment

Uh oh!

gabrieldagama May 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gabrieldagama May 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nazar65 commented May 21, 2020

Uh oh!

Nazar65 commented May 21, 2020

Uh oh!

Nazar65 commented May 21, 2020

Uh oh!

Nazar65 commented May 21, 2020

Uh oh!

magento-engcom-team commented May 21, 2020

Uh oh!

engcom-Bravo commented May 22, 2020

Uh oh!

chalov-anton commented May 25, 2020

Uh oh!

m2-assistant bot commented May 28, 2020

Uh oh!

Uh oh!

Nazar65 commented May 14, 2020 •

edited by slavvka

Loading

gabrieldagama May 18, 2020 •

edited

Loading

gabrieldagama May 18, 2020 •

edited

Loading