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.

Sign up for GitHub

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

Jump to bottom

Added Content Status and Store View filters #1602

Merged
merged 10 commits into from
Jul 21, 2020
Merged

Added Content Status and Store View filters #1602

merged 10 commits into from
Jul 21, 2020

Conversation

gabrieldagama
Copy link
Contributor

@gabrieldagama gabrieldagama commented Jul 16, 2020 •
edited
Loading

Description (*)

Added Content Status and Store View filters.

  1. Depends on Added GetAssetIdByContentFieldInterface and implementations magento2#29058

Related Pull Requests

magento/magento2#29058

Fixed Issues (if relevant)

  1. Fixes Introduce Store view filter #1463
  2. Fixes Introduce Enabled filter #1464

Manual testing scenarios (*)

  1. Assign an asset to a content (product, category, page or block)
  2. Go to Content -> Media Gallery and filter by Content Status or Store View
  3. The filter output should be assets that are used on content and the content matches the filter condition.

@gabrieldagama
Copy link
Contributor Author

@magento run all tests

@gabrieldagama
Copy link
Contributor Author

@magento run all tests

2 similar comments
@gabrieldagama
Copy link
Contributor Author

@magento run all tests

@gabrieldagama
Copy link
Contributor Author

@magento run all tests

@gabrieldagama
Copy link
Contributor Author

@magento run all tests

chalov-anton
chalov-anton reviewed Jul 17, 2020
View reviewed changes
MediaGalleryUi/Test/Mftf/Test/AdminMediaGalleryDisabledContentFilterTest.xml Outdated
<useCaseId value="https://github.com/magento/adobe-stock-integration/issues/1464"/>
<title value="User filter asset by disabled content"/>
<stories value="Story 57: User filters images by the area they used in"/>
<testCaseId value="to-be-added"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<testCaseId value="to-be-added"/>
<testCaseId value="https://studio.cucumber.io/projects/131313/test-plan/folders/1320712/scenarios/4970565"/>

sivaschenko
sivaschenko requested changes Jul 17, 2020
View reviewed changes
Copy link
Member

@sivaschenko sivaschenko left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @gabrieldagama , please see my comments

MediaGalleryUi/Model/SearchCriteria/CollectionProcessor/FilterProcessor/ContentField.php Outdated
*/
class ContentField implements CustomFilterInterface
{
private const TABLE_ALIAS = 'main_table';
Copy link
Member

Choose a reason for hiding this comment

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

A bit of inconsistency: there is a constant for the table name but no constant for id field name. I'd have both on none

MediaGalleryUi/Test/Mftf/ActionGroup/AdminMediaGalleryApplySelectFilterActionGroup.xml Outdated
</annotations>
<arguments>
<argument name="selectLabel" type="string"/>
<argument name="selectOption" type="string"/>
Copy link
Member

Choose a reason for hiding this comment

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

filterLabel and optionLabel argument names would be more descriptive for a developer

MediaGalleryUi/Test/Mftf/Section/AdminEnhancedMediaGalleryFiltersSection.xml Outdated Show resolved Hide resolved
...aGalleryUi/Test/Mftf/ActionGroup/AdminOpenMediaGalleryForExistingPageNoEditorActionGroup.xml Outdated Show resolved Hide resolved
MediaGalleryUi/Test/Mftf/Test/AdminMediaGalleryStoreViewContentFilterTest.xml Show resolved Hide resolved
modman Outdated Show resolved Hide resolved
@gabrieldagama
Copy link
Contributor Author

@magento run all tests

sivaschenko
sivaschenko previously approved these changes Jul 21, 2020
View reviewed changes
@sivaschenko
Copy link
Member

@magento run all tests

sivaschenko
sivaschenko previously approved these changes Jul 21, 2020
View reviewed changes
@gabrieldagama
Copy link
Contributor Author

@magento run all tests

Gabriel Galvao da Gama added 2 commits July 21, 2020 13:55
Merge branch '2.0-develop' of github.com:magento/adobe-stock-integrat…
c257842 
…ion into introduce-enable-store-view-filters
Merge branch 'introduce-enable-store-view-filters' of github.com:gabr…
1eefe4c 
…ieldagama/adobe-stock-integration into introduce-enable-store-view-filters
@gabrieldagama
Copy link
Contributor Author

@magento run all tests

@chalov-anton
Copy link
Contributor

✔️ Great job

The Store view filter handles multiple Sites and Views perfectly
store_view

The Content status filter works for Enabled and Disabled Pages, Blocks, Products, Categories

The filters work together with other already existing filters

@m2-community-project m2-community-project bot added Severity: S2 Affects non-critical data or functionality and forces users to employ a workaround Priority: P2 Should be fixed as early as possible and removed Severity: S2 Affects non-critical data or functionality and forces users to employ a workaround labels Jul 21, 2020
@gabrieldagama gabrieldagama added Priority: P2 Should be fixed as early as possible and removed Priority: P2 Should be fixed as early as possible labels Jul 21, 2020
@m2-community-project m2-community-project bot added the Severity: S2 Affects non-critical data or functionality and forces users to employ a workaround label Jul 21, 2020
@gabrieldagama gabrieldagama removed Severity: S2 Affects non-critical data or functionality and forces users to employ a workaround Priority: P2 Should be fixed as early as possible labels Jul 21, 2020
@m2-community-project m2-community-project bot added the Priority: P2 Should be fixed as early as possible label Jul 21, 2020
@gabrieldagama gabrieldagama merged commit 535dbb4 into magento:2.0-develop Jul 21, 2020
@ghost
Copy link

ghost commented Jul 21, 2020

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.

@sivaschenko sivaschenko mentioned this pull request Jul 22, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chalov-anton chalov-anton chalov-anton left review comments

@sivaschenko sivaschenko sivaschenko left review comments

Assignees

@sivaschenko sivaschenko

@chalov-anton chalov-anton

Labels
Priority: P2 Should be fixed as early as possible
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Introduce Enabled filter Introduce Store view filter
4 participants
@gabrieldagama @sivaschenko @chalov-anton @lenaorobei