laminas / laminas-filter

Programmatically filter and normalize data and files
https://docs.laminas.dev/laminas-filter/
BSD 3-Clause "New" or "Revised" License
85 stars 35 forks source link

WIP - Add more tests and start refactoring MonthSelect #200

Open ramchale opened 1 week ago

ramchale commented 1 week ago
Q A
Documentation yes
Bugfix no
BC Break yes
New Feature no
RFC yes
QA no

Description

Draft pull request to discuss desired behavior of MonthSelect in v3. Some of this will also be relevant to other classes extending AbstractDateDropdown.

gsteel commented 1 week ago

Looks to me like expected input is, like you say, an associative array of varying lengths depending on the descendant class that should produce a formatted string. The filter requires that array members be specificly year, month etc, so it's already very restrictive - I really don't see the point of this filter at all - I mean, on the frontend, why not just use an <input type="date|datetime-local"> - this filter pre-dates HTML5, so I'm wondering if we should just deprecate all of it and remove it completely.

cc @froschdesign

ramchale commented 1 week ago

Cool. Does that apply to the other two that inherit from AbstractDateDropdown (DateSelect and DateTimeSelect), as they share a lot of the same behavior at the moment?

gsteel commented 1 week ago

I'd say all 4 classes, MonthSelect, Abstract, DateSelect & DateTimeSelect - Let's wait for @froschdesign to give us an opinion too đź‘Ť

froschdesign commented 1 week ago

I'd say all 4 classes, MonthSelect, Abstract, DateSelect & DateTimeSelect…

Unfortunately, that won't work, because laminas-form uses these filters:

ramchale commented 6 days ago

I'd say all 4 classes, MonthSelect, Abstract, DateSelect & DateTimeSelect…

Unfortunately, that won't work, because laminas-form uses these filters:

* `Laminas\Form\Element\DateSelect`: https://github.com/laminas/laminas-form/blob/653c869d10c361027ae6c660c991ec3e3f38ed65/src/Element/DateSelect.php#L175-L193

* `Laminas\Form\Element\DateTimeSelect`: https://github.com/laminas/laminas-form/blob/653c869d10c361027ae6c660c991ec3e3f38ed65/src/Element/DateTimeSelect.php#L335-L353

* `Laminas\Form\Element\MonthSelect`: https://github.com/laminas/laminas-form/blob/653c869d10c361027ae6c660c991ec3e3f38ed65/src/Element/MonthSelect.php#L325-L343

Thanks both. Does that bring us back to the questions about behavior in v3 or is there more to consider?

gsteel commented 6 days ago

OK, so I think that in all erroneous cases, the filter(s) should return the unprocessed input that will hopefully then fail subsequent validation - so, out of range, incorrect array shapes etc should not throw exceptions, but just silently pass through unchanged.

I think we have to work on the premise that we're working with data that hasn't been validated. In typical usage with input-filter, throwing an exception for malformed input prevents validators from doing their job and reporting to the user that the input was rejected.

froschdesign commented 6 days ago

OK, so I think that in all erroneous cases, the filter(s) should return the unprocessed input that will hopefully then fail subsequent validation - so, out of range, incorrect array shapes etc should not throw exceptions, but just silently pass through unchanged.

Based on the form elements from above: The validators Date and Regex can handle the unprocessed input arrays so that the validators can fulfil their jobs in all cases and produce the related error messages.