Added GetAssetIdByContentFieldInterface and implementations #29058

gabrieldagama · 2020-07-09T14:11:52Z

Description (*)

Added GetAssetIdByContentFieldInterface and its implementation on MediaContent modules, that will allow Adobe Stock Integration to extend MediaGallery filter functionality.

Integration tests to be added.

Related Pull Requests

https://github.com/magento/partners-magento2-infrastructure/pull/10

Fixed Issues (if relevant)

Manual testing scenarios (*)

...
...

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-07-09T14:11:55Z

Hi @gabrieldagama. 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 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:

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

You can find more information about the builds here

ℹ️ 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.

sivaschenko

Thanks for the PR @gabrieldagama ! Looks nice! Please see my comments

app/code/Magento/MediaContentCms/Model/GetAssetIdByContentField.php

app/code/Magento/MediaContentApi/Model/GetAssetIdByContentFieldInterface.php

app/code/Magento/MediaContent/etc/di.xml

gabrieldagama · 2020-07-10T15:00:47Z

@magento run all tests

sivaschenko

Thanks for updates @gabrieldagama ! Looks nice! Please checkout my review comments

app/code/Magento/MediaContentApi/Api/GetAssetIdByContentFieldInterface.php

app/code/Magento/MediaContent/Model/GetAssetIdByContentFieldComposite.php

app/code/Magento/MediaContentCatalog/Model/GetAssetIdByCategoryStore.php

app/code/Magento/MediaContent/Model/GetAssetIdByContentFieldComposite.php

app/code/Magento/MediaContentApi/Model/GetAssetIdByContentFieldInterface.php

...ation/testsuite/Magento/MediaContentCms/Model/ResourceModel/GetAssetIdByContentFieldTest.php

app/code/Magento/MediaContentCatalog/Model/GetAssetIdByEavContentField.php

sivaschenko

Thanks for updates @gabrieldagama ! Please see my review comments

sivaschenko · 2020-07-16T10:47:16Z

app/code/Magento/MediaContentApi/Model/Composite/GetAssetIdsByContentField.php

+            throw new InvalidArgumentException(__('The field argument is invalid.'));
+        }
+        $ids = [];
+        /** @var GetAssetIdsByContentFieldInterface $fieldHandlers */


unnecessary annotation (the type is already declared on property)

I will actually update the property, that is not correct, it is a multidimensional array, instead of an array of GetAssetIdsByContentFieldInterface.

app/code/Magento/MediaContentApi/Model/Composite/GetAssetIdsByContentField.php

sivaschenko · 2020-07-16T10:51:32Z

app/code/Magento/MediaContentCatalog/Model/ResourceModel/GetAssetIdsByCategoryStore.php

+        )->where(
+            'entity_id IN (?)',
+            $categoryIds
+        );


Can we simplify the algorithm and abovid additional query to retrieve the category ids?

Suggested change

)->where(

'entity_id IN (?)',

$categoryIds

);

)->where(

'path like ?',

$storeGroup->getRootCategoryId() . '%'

);

Nice catch, I guess it looks way better now, thanks!

gabrieldagama · 2020-07-17T10:06:27Z

@magento run all tests

gabrieldagama · 2020-07-17T13:06:33Z

@magento run all tests

magento-engcom-team · 2020-07-20T10:32:19Z

Hi @sivaschenko, thank you for the review.
ENGCOM-7858 has been created to process this Pull Request
✳️ @sivaschenko, 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

gabrieldagama · 2020-07-20T10:33:20Z

@magento run all tests

sivaschenko · 2020-07-20T10:33:52Z

This PR introduces a service that is covered by integration tests, QA activities are not necessary

…ons #29058

m2-assistant · 2020-07-21T10:47:52Z

Hi @gabrieldagama, 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.

Gabriel Galvao da Gama added 8 commits July 7, 2020 12:02

Implemented content status filter

dfc733a

Improved attribute fetching

3a1a3c7

Improved php doc blocks

15aa96f

Added UI filter to media gallery

1fcb11a

Refactor and added store view filter

a7e980c

Removed MediaContentUi from core repo

279cf2d

Refactor of category store class and removed unecessary stuff

11082e7

Removed contruct params and added constants

e0a777b

magento-engcom-team added Component: MediaContent Component: MediaContentApi Component: MediaContentCatalog Component: MediaContentCms labels Jul 9, 2020

ghost added the Progress: pending review label Jul 9, 2020

magento-engcom-team added the Release Line: 2.4 label Jul 9, 2020

gabrieldagama requested a review from sivaschenko July 9, 2020 14:12

sivaschenko requested changes Jul 9, 2020

View reviewed changes

ghost assigned sivaschenko Jul 9, 2020

ghost added Progress: needs update and removed Progress: pending review labels Jul 9, 2020

Gabriel Galvao da Gama added 2 commits July 10, 2020 11:12

Implemented suggestions

8955147

Added integration tests

545f886

gabrieldagama changed the title ~~[WIP] Added GetAssetIdByContentFieldInterface and implementations~~ Added GetAssetIdByContentFieldInterface and implementations Jul 10, 2020

gabrieldagama marked this pull request as ready for review July 10, 2020 15:00

sivaschenko requested changes Jul 10, 2020

View reviewed changes

Gabriel Galvao da Gama added 2 commits July 13, 2020 10:44

Updated names

5f82525

Updated test names

aa11cf5

sivaschenko requested changes Jul 16, 2020

View reviewed changes

ghost assigned gabrieldagama Jul 17, 2020

Implemented suggestions

193765d

Added try to cover case when root category doesnt exists

095dcc6

gabrieldagama added Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Severity: S2 Major restrictions or short-term circumventions are required until a fix is available. labels Jul 17, 2020

sivaschenko approved these changes Jul 20, 2020

View reviewed changes

ghost added Progress: ready for testing and removed Progress: needs update labels Jul 20, 2020

sivaschenko added the Auto-Tests: Covered All changes in Pull Request is covered by auto-tests label Jul 20, 2020

ghost removed the Progress: ready for testing label Jul 20, 2020

ghost added Progress: testing in progress and removed Progress: testing in progress labels Jul 20, 2020

ghost added Progress: extended testing Progress: accept and removed Progress: extended testing labels Jul 20, 2020

magento-engcom-team mentioned this pull request Jul 21, 2020

Two invoices to the entire order #1464

Closed

magento-engcom-team pushed a commit that referenced this pull request Jul 21, 2020

ENGCOM-7858: Added GetAssetIdByContentFieldInterface and implementati…

e3471b8

…ons #29058

magento-engcom-team merged commit 3787390 into magento:2.4-develop Jul 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added GetAssetIdByContentFieldInterface and implementations #29058

Added GetAssetIdByContentFieldInterface and implementations #29058

gabrieldagama commented Jul 9, 2020 •

edited

Loading

m2-assistant bot commented Jul 9, 2020

sivaschenko left a comment

gabrieldagama commented Jul 10, 2020

sivaschenko left a comment

sivaschenko left a comment

sivaschenko Jul 16, 2020

gabrieldagama Jul 17, 2020 •

edited

Loading

sivaschenko Jul 16, 2020

gabrieldagama Jul 17, 2020

gabrieldagama commented Jul 17, 2020

gabrieldagama commented Jul 17, 2020

magento-engcom-team commented Jul 20, 2020

gabrieldagama commented Jul 20, 2020

sivaschenko commented Jul 20, 2020

m2-assistant bot commented Jul 21, 2020

Added GetAssetIdByContentFieldInterface and implementations #29058

Added GetAssetIdByContentFieldInterface and implementations #29058

Conversation

gabrieldagama commented Jul 9, 2020 • edited Loading

Description (*)

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

m2-assistant bot commented Jul 9, 2020

sivaschenko left a comment

Choose a reason for hiding this comment

gabrieldagama commented Jul 10, 2020

sivaschenko left a comment

Choose a reason for hiding this comment

sivaschenko left a comment

Choose a reason for hiding this comment

sivaschenko Jul 16, 2020

Choose a reason for hiding this comment

gabrieldagama Jul 17, 2020 • edited Loading

Choose a reason for hiding this comment

sivaschenko Jul 16, 2020

Choose a reason for hiding this comment

gabrieldagama Jul 17, 2020

Choose a reason for hiding this comment

gabrieldagama commented Jul 17, 2020

gabrieldagama commented Jul 17, 2020

magento-engcom-team commented Jul 20, 2020

gabrieldagama commented Jul 20, 2020

sivaschenko commented Jul 20, 2020

m2-assistant bot commented Jul 21, 2020

gabrieldagama commented Jul 9, 2020 •

edited

Loading

gabrieldagama Jul 17, 2020 •

edited

Loading