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.

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

Rename Api\SearchCriteriaBuilder::addFilter() to addFilters() #1421

Merged
merged 1 commit into from
Jul 15, 2015
Merged

Rename Api\SearchCriteriaBuilder::addFilter() to addFilters() #1421

merged 1 commit into from
Jul 15, 2015

Commits on Jun 28, 2015

  1. Rename \Magento\Framework\Api\SearchCriteriaBuilder::addFilter() to a…

    …ddFilters()
    
    Purpose
    ==
    
    Make method name more descriptive and easier to understand.
    
    Background
    ==
    
    Having a method called `addFilter()` which take an array of `Filter` instances caused
    many doubletakes and facepalms during the m2 dev training.
    It seems as if the method name originated from the old collection `addFieldToFilter`
    from collections in Magento 1, where the condition type was set as a key in the passed
    argument array (e.g. `addFieldToFilter('name', ['eq' => $filter])`).
    In fact, some integration tests even set such expectations on the searchCriteriaBuilder,
    for example *app/code/Magento/Sales/Test/Unit/Model/Service/CreditmemoServiceTest.php*.
    
    However, this must be a bug because the array keys in the argument array are ignored,
    and the code only happened to work because the repositories in question default to
    using the 'eq' operator when applying the filter groups to the collection if no condition
    type is set on the filter instance.
    
    Reasons for this PR
    ==
    
    Make the method name more intuitive.
    Naming definitely could still be improved, but at least it no longer is that misleading.
    Maybe using `addFilterGroupWithFilters()` would be even more descriptive, but I chose the
    smaller change, since it probably is "good enough".
    
    Remove wrong examples of method usage from tests, so they don't serve as wrong
    documentation.
    Vinai committed Jun 28, 2015
    Configuration menu
    Copy the full SHA
    4527e32 View commit details
    Browse the repository at this point in the history