Filters priority setting when populating filters in inputfilter factory and not losing it when merging filter chains #4148

Closed
wants to merge 4 commits into
from

Projects

None yet

2 participants

@leup
Contributor
leup commented Mar 30, 2013

This PR do two things.

First, we can set the filter priority in the configuration array when creating an input filter through factory.

$input->add($factory->createInput(array(
    'name' => 'img_background',
    'required' => true
    'filters' => array(
        array(
            'name' => 'Admin\Filter\Diacritics',
            'priority' => 10000
        ),
    ),
)));

It works also for filters set within the form element, here:

class Image extends \Zend\Form\File
{
    public function getInputSpecification() {
        $specs = parent::getInputSpecification();
        if (!isset($specs['filters']) || !is_array($specs['filters'])) {
            $specs['filters'] = array();
        }
        $specs['filters'][] = array(
            'name' => '\Admin\Filter\Thumbnail',
            'priority' => 999
        );

        return $specs;
    }
}

Second, when merging two filter chains, the filters priority of the merged filter chain are no more lost in the process (reset to the default_priority constant which is 1000 for now).

Keeping my examples above if this two filter chains were related to the same form element then they would be merged and the filters priority of the merged filter chain would be lost (in my example, the merged one is the one with the Diacritics Filter).

With this PR the priority of the Diacritics filter would not be lost when the two filter chains would be merged.

PS: I wanted to make some unit tests but the truth is I don't even know where to start, I'm not even sure it is needed.

@leup
Contributor
leup commented Apr 8, 2013

any comment on this ?

@weierophinney
Member

Please provide unit tests so we can (a) confirm that the issue exists, (b) confirm that the issue is fixed with your code, and (c) ensure we don't introduce a regression that re-introduces the issue at a later date.

Thanks!

leup added some commits Apr 12, 2013
@leup leup Add Filter\FilterChain unit test to test if filter priorities are kep…
…t when merging one filterChain into another.
f41da83
@leup leup Add InputFilter\Factory unit test to check that the priority setting …
…in the filters key array are passed to the FilterChain
d3f36b1
@leup
Contributor
leup commented Apr 12, 2013

I wrote two unit tests and run them against the current master branch and my PR.
The two tests failed against master and passed against PR.
As I am new to that, please advise.

@weierophinney
Member

That's exactly as it should work - the new code is not on master, so test
fail; with your PR, they pass. :-)

On Friday, April 12, 2013, paul@gh wrote:

I wrote two unit tests and run them against the current master branch and
my PR.
The two tests failed against master and passed against PR.
As I am new to that, please advise.


Reply to this email directly or view it on GitHubhttps://github.com/zendframework/zf2/pull/4148#issuecomment-16314562
.

Matthew Weier O'Phinney
matthew@weierophinney.net
http://mwop.net/

@weierophinney weierophinney added a commit that referenced this pull request Apr 15, 2013
@weierophinney weierophinney [#4148] CS fixes
- per php-cs-fixer
a3a9b07
@weierophinney weierophinney added a commit that referenced this pull request Apr 15, 2013
@weierophinney weierophinney Merge branch 'hotfix/4148' into develop
Forward port #4148
59be48b
@weierophinney weierophinney added a commit that closed this pull request Apr 15, 2013
@weierophinney weierophinney Merge branch 'hotfix/4148'
Close #4148
d189381
@weierophinney weierophinney added a commit to zendframework/zend-inputfilter that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge pull request zendframework/zendframework#4148 from leup/filters…
…priority

Filters priority setting when populating filters in inputfilter factory and not losing it when merging filter chains
9c65c45
@weierophinney weierophinney added a commit to zendframework/zend-inputfilter that referenced this pull request May 15, 2015
@weierophinney weierophinney [zendframework/zendframework#4148] CS fixes
- per php-cs-fixer
24475df
@weierophinney weierophinney added a commit to zendframework/zend-inputfilter that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'hotfix/4148' 1d32d23
@weierophinney weierophinney added a commit to zendframework/zend-inputfilter that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'hotfix/4148' into develop af70035
@weierophinney weierophinney added a commit to zendframework/zend-filter that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge pull request zendframework/zendframework#4148 from leup/filters…
…priority

Filters priority setting when populating filters in inputfilter factory and not losing it when merging filter chains
663703c
@weierophinney weierophinney added a commit to zendframework/zend-filter that referenced this pull request May 15, 2015
@weierophinney weierophinney [zendframework/zendframework#4148] CS fixes
- per php-cs-fixer
03b22d8
@weierophinney weierophinney added a commit to zendframework/zend-filter that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'hotfix/4148' 61d3103
@weierophinney weierophinney added a commit to zendframework/zend-filter that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'hotfix/4148' into develop 1fed17b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment