fixxed #94 'Missing required failure message for CollectionInputFilter' issue

[ruzann]

Added default message to display when a collection is required.
Added mechanism for setting a custom "required" message.
Updated factorie to allow setting that message.

[ruzann]

@froschdesign Is there any updates relation to pull request?

[froschdesign]

@ruzann
I still see a problem here, because your changes introduce a hard coded error message to an input-filter, that means we have message constants in input-filters and validators.
This ends in another problem, because the error message can not be translated.

[froschdesign]

@ruzann
Do not create a new translator object, this will introduce new problems and you must set the the translation files for the input-filter - this is wrong!
Please revert this and use the translator from the validator.

[ruzann]

@froschdesign can you please review?

[ruzann]

@froschdesign all your notes fixed, please check again, when you have a time.

[newdevonair]

@froschdesign is there any news from this pull request?
I need this functionality in my project too.

Thanks.

[froschdesign]

@ruzann
Some pseudo code:

function getNotEmptyValidator()
{
    if ($this->notEmptyValidator === null) {
        $this->notEmptyValidator = new NotEmpty();
    }

    return $this->notEmptyValidator;
}

function setNotEmptyValidator($notEmptyValidator);

function prepareRequiredValidationFailureMessage();

The default error message already exists and you can replace the validator with your own. messageRequired and the related methods are not needed.

(Yes, we do not use the validator for validation, but we allow a translation of the error message with the standard way.)

What do you think?

/cc @malukenho @rkeet

@newdevonair
You can also add a review or a comment to the current suggestions. 😉

---

Sorry for the late response and the short review. My focus is currently on the documentation, website and zend-navigation.

[rkeet]

I would suggest same as @froschdesign review: own getter/setter for storing NotEmpty instance.

Otherwise seems alright to me, apart from tests not passing ;)

[ruzann]

@froschdesign I fixed your notes, please check again.

[tigran-m-dev]

Hi guys, do you have any updates related this issue? Is it ready to merge or have any other improvement?
If you need some help I am ready to help.

[weierophinney]

Honestly, this looks really convoluted. If I'm reading this correctly:

• You allow injecting a string message for the purposes of seeding a NotEmpty validator
• You manually test if the collection is empty, and, if so:
  • grab the NotEmpty validator
  • Grab its NOT_EMPTY template to use as a message
  • Optionally translate the message, if the validator has a translator attached
  • Return that message

It seems like it would make more sense to:

• Allow setting a message template for when the collection is empty
• Allow optionally setting a translator, to allow translating the above message
• Use each of the above when the collection is empty to populate the validation failure messages

Alternately, we should allow specifying a validator and/or validator chain at the top level for checking the status of the collection, prior to attempting to validate the members of the collection.

[froschdesign]

@weierophinney

Alternately, we should allow specifying a validator and/or validator chain at the top level for checking the status of the collection, prior to attempting to validate the members of the collection.

This one! (See my example with pseudo code above.)
Why? In all inputs we uses the messages from the validator. Setting a message to an input would introduce a new behaviour for this component. Setting a validator or a validator chain is not new.
The validator has already the option to translate a message.

[ruzann]

@weierophinney, @froschdesign is there any news?
should I change something?

[weierophinney]

@ruzann Please see the feedback above - @froschdesign and I feel that the approach outlined here is convoluted, and inconsistent with the architecture. Adding a validator chain specific to the collection input filter as an entity would make more sense. If you can refactor to that approach, we can review again.

[froschdesign]

Please use required_message for consistency.

[ruzann]

@froschdesign is there any news?

[froschdesign]

@ruzann
Your improvement will be included in the next minor release (2.9). At the moment I am working at documentation of zend-hydrator, therefore I can not give a time frame. I'm sorry.

In the meantime, just use your own fork.

[weierophinney]

The changes look much better, @ruzann .

I'm going to do the following:

• Rebase off of develop
• Squash your commits (50 is excessive here!)
• Split the tests up to demonstrate the behaviors we're interested in
• Push a few additional changes I noticed during review

Then we can 🚢!

[weierophinney]

Thanks, @ruzann!