magento/adobe-stock-integration#1467: Introduce Used In Pages filter … #1546

ghost · 2020-06-29T10:08:37Z

Description (*)

Implement Used In Pages filter. user can filter images by CMS pages

Fixed Issues (if relevant)

Fixes Introduce Used In Pages filter #1467: Introduce Used In Pages filter

Manual testing scenarios (*)

Add image to a specific Cms page
Admin > content > media gallery > click filter & select Pages

…r. user can filter images by pages

ghost · 2020-06-29T10:09:14Z

@magento run all tests

coderimus

Hi @gregbalonzo ! Thank you for the work you have done in this PR 👍 Please, check my change requests comments below.

MediaGalleryUi/Model/SearchCriteria/CollectionProcessor/FilterProcessor/EntityId.php

coderimus · 2020-06-29T11:59:20Z

MediaGalleryUi/Model/SearchCriteria/CollectionProcessor/JoinProcessor/EntityId.php

+            []
+        );
+
+        //print_r($collection->getSelect()->__toString()); die;


Please, remove this line.

MediaGalleryUi/Ui/Component/Listing/Columns/Pages/Options.php

coderimus · 2020-06-29T12:03:39Z

MediaGalleryUi/Ui/Component/Listing/Columns/Pages/Options.php

+    {
+        $searchCriteria = $this->_searchCriteriaBuilder->create();
+        $pages = [];
+


Please, remove this empty line.

coderimus · 2020-06-29T12:04:34Z

MediaGalleryUi/Ui/Component/Listing/Columns/Pages/Options.php

+                ];
+            }
+        } catch (LocalizedException $e) {
+


Think that exception should be thrown in that case. Please, replace $e with $exception

@gregbalonzo thanks for renaming a variable. Why do we need to suppress the exception, though?

@sivaschenko its for my local testing purpose, I can remove it if this is not required.

@gregbalonzo please keep the code for local testing purposes on the local, try to go only with the production-ready code for the push to Github.

MediaGalleryUi/Model/SearchCriteria/CollectionProcessor/JoinProcessor/EntityId.php

MediaGalleryUi/Ui/Component/Listing/Columns/Pages/Options.php

…Processor/EntityId.php Co-authored-by: Alexander Shkurko <[email protected]>

…(strict_types=1); & remove empty line

MediaGalleryUi/view/adminhtml/ui_component/standalone_media_gallery_listing.xml

Nazar65 · 2020-06-29T17:03:47Z

MediaGalleryUi/Ui/Component/Listing/Columns/Pages/Options.php

+/**
+ * Image source filter options
+ */
+class Options implements OptionSourceInterface


Description can be updated, to the page filter options

MediaGalleryUi/view/adminhtml/ui_component/standalone_media_gallery_listing.xml

…hange placeholder

@param

…ld have a description, change protected to private, Add the constructor @param section

coderimus · 2020-07-01T14:20:37Z

@magento run all tests

sivaschenko

Thanks for the PR @gregbalonzo ! Please see my review comments

sivaschenko · 2020-07-02T12:17:02Z

MediaGalleryUi/Model/SearchCriteria/CollectionProcessor/FilterProcessor/EntityId.php

+/**
+ * Class EntityId
+ * Used to filter the collection based on the filter selection.
+ * @package Magento\MediaGalleryUi\Model\SearchCriteria\CollectionProcessor\FilterProcessor


Please remove the Class EntityId and @package lines from the class comments (applicable to all classes)

sivaschenko · 2020-07-02T12:20:49Z

MediaGalleryUi/Model/SearchCriteria/CollectionProcessor/JoinProcessor/EntityId.php

+     */
+    public function apply(AbstractDb $collection): bool
+    {
+        $collection->getSelect()->joinLeft(


This join is going to be added in #1531 no need to join the same table twice

sivaschenko · 2020-07-02T12:21:39Z

MediaGalleryUi/Model/SearchCriteria/CollectionProcessor/JoinProcessor/EntityId.php

+ * Used to join table of the collection based on the filter selection.
+ * @package Magento\MediaGalleryUi\Model\SearchCriteria\CollectionProcessor\JoinProcessor
+ */
+class EntityId implements CustomJoinInterface


Please rename the class to Page to better reflect it's responsibility

sivaschenko · 2020-07-02T12:22:15Z

MediaGalleryUi/Ui/Component/Listing/Columns/Pages/Options.php

+use Magento\Framework\Data\OptionSourceInterface;
+use Magento\Framework\Exception\LocalizedException;
+
+


unnecessary empty line

sivaschenko · 2020-07-02T12:24:30Z

MediaGalleryUi/Ui/Component/Listing/Columns/Pages/Options.php

+                ];
+            }
+        } catch (LocalizedException $e) {
+


@gregbalonzo thanks for renaming a variable. Why do we need to suppress the exception, though?

sivaschenko · 2020-07-02T12:25:11Z

MediaGalleryUi/Ui/Component/Listing/Columns/Pages/Options.php

+    /**
+     * @inheritdoc
+     */
+    public function toOptionArray(): array


It would be great to load the items dynamically, based on the user input

…de & renamed php class

…uery because its not accurate when searching by pages

…eryUi/Ui/Component/Listing/Columns/Pages/Options.php

gabrieldagama

Hi @gregbalonzo, thanks for the PR! Please follow some comments below:

MediaGalleryUi/view/adminhtml/ui_component/standalone_media_gallery_listing.xml

MediaGalleryUi/Model/SearchCriteria/CollectionProcessor/JoinProcessor/Page.php

gabrieldagama · 2020-07-06T09:28:57Z

MediaGalleryUi/Ui/Component/Listing/Columns/Pages/Options.php

+ */
+class Options implements OptionSourceInterface
+{
+    private $PageRepository;


Missing php doc block and please make this camelCase.

gabrieldagama · 2020-07-06T09:29:00Z

MediaGalleryUi/Ui/Component/Listing/Columns/Pages/Options.php

+{
+    private $PageRepository;
+
+    private $SearchCriteriaBuilder;


Missing php doc block and please make this camelCase.

gabrieldagama · 2020-07-06T09:29:05Z

MediaGalleryUi/Ui/Component/Listing/Columns/Pages/Options.php

+
+    private $SearchCriteriaBuilder;
+
+    private $FilterBuilder;


Missing php doc block and please make this camelCase.

…the join query and replace same as used in used in filter

sivaschenko · 2020-07-13T14:24:38Z

Closed in favour to #1593

ghost · 2020-07-13T14:24:40Z

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

#1467: Introduce Used In Pages filter - Implement Used In Pages filte…

74016c8

…r. user can filter images by pages

coderimus suggested changes Jun 29, 2020

View reviewed changes

m2-community-project bot assigned coderimus Jun 29, 2020

m2-community-project bot added the Progress: changes requested label Jun 29, 2020

killerboduk1 and others added 2 commits June 29, 2020 20:09

Update MediaGalleryUi/Model/SearchCriteria/CollectionProcessor/Filter…

de631d7

…Processor/EntityId.php Co-authored-by: Alexander Shkurko <[email protected]>

#1467: Introduce Used In Pages filter - fix PR comment remove declare…

0e96e8e

…(strict_types=1); & remove empty line

Nazar65 suggested changes Jun 29, 2020

View reviewed changes

killerboduk1 added 2 commits June 30, 2020 10:19

#1467: Introduce Used In Pages filter - Resolve PR commented block, c…

40ed2f0

…hange placeholder

#1467: Introduce Used In Pages filter - resolve PR comment class shou…

3a302cc

…ld have a description, change protected to private, Add the constructor @param section

sivaschenko requested changes Jul 2, 2020

View reviewed changes

m2-community-project bot assigned sivaschenko Jul 2, 2020

killerboduk1 added 3 commits July 3, 2020 20:02

#1467: Introduce Used In Pages filter - remove unnecessary line of co…

698c9c3

…de & renamed php class

#1467:Introduce Used In Pages filter - did some modification on sql q…

e99fd6c

…uery because its not accurate when searching by pages

#1467:Introduce Used In Pages filter - remove empty line on MediaGall…

41b265f

…eryUi/Ui/Component/Listing/Columns/Pages/Options.php

gabrieldagama suggested changes Jul 6, 2020

View reviewed changes

m2-community-project bot assigned gabrieldagama Jul 6, 2020

#1467:Introduce Used In Pages filter - resolve PR comment and remove …

7b8c66c

…the join query and replace same as used in used in filter

sivaschenko closed this Jul 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

magento/adobe-stock-integration#1467: Introduce Used In Pages filter … #1546

magento/adobe-stock-integration#1467: Introduce Used In Pages filter … #1546

ghost commented Jun 29, 2020

ghost commented Jun 29, 2020

coderimus left a comment

coderimus Jun 29, 2020

coderimus Jun 29, 2020

coderimus Jun 29, 2020

sivaschenko Jul 2, 2020

ghost Jul 3, 2020

sivaschenko Jul 4, 2020

Nazar65 Jun 29, 2020

coderimus commented Jul 1, 2020

sivaschenko left a comment

sivaschenko Jul 2, 2020

sivaschenko Jul 2, 2020

sivaschenko Jul 2, 2020

sivaschenko Jul 2, 2020

sivaschenko Jul 2, 2020

sivaschenko Jul 2, 2020

gabrieldagama left a comment

gabrieldagama Jul 6, 2020

gabrieldagama Jul 6, 2020

gabrieldagama Jul 6, 2020

sivaschenko commented Jul 13, 2020

ghost commented Jul 13, 2020

		use Magento\Framework\Data\OptionSourceInterface;
		use Magento\Framework\Exception\LocalizedException;

magento/adobe-stock-integration#1467: Introduce Used In Pages filter … #1546

magento/adobe-stock-integration#1467: Introduce Used In Pages filter … #1546

Conversation

ghost commented Jun 29, 2020

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

ghost commented Jun 29, 2020

coderimus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coderimus commented Jul 1, 2020

sivaschenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabrieldagama left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sivaschenko commented Jul 13, 2020

ghost commented Jul 13, 2020