Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Change the way a field is retrieved for fixing multiple field bug. #563

Closed
wants to merge 1 commit into from

3 participants

@stefanozoffoli

I found a problem when using FormHelper to create input fields with multiple values.

Here's an example:

<?php for ($i = 0, $n = count($this->request->data['Contact']['others']); $i <= $n; $i++): ?>
    <?php
        echo $this->Form->input('Contact.others.' . $i . '.0', array(
            'label'         => __('Key'),
            'type'          => 'text',
        ));
    ?>
    <?php
        echo $this->Form->input('Contact.others.' . $i . '.1', array(
            'label'         => __('Value'),
            'type'          => 'text',
        ));
    ?>
<?php endfor; ?>

Without this patch, cakephp put "required" class in all "Contact.others.n.0" divs, even if I didn't create a "validate" rule for that key.

In fact, when FormHelper->input() calls Helper->field() method to determine the name of the field, that method doesn't strip all the numbers of a multiple field key.
In this case, for all the "Contact.others.n.0" Helper->field() returns "0", and FormHelper->_introspectModel() returns true because of this:

if ($key === 'validates') {
    if (empty($field)) {
        return $this->fieldset[$model]['validates'];
    } else {
        return isset($this->fieldset[$model]['validates'][$field]) ?
            $this->fieldset[$model]['validates'] : null;
    }
}

All the inputs get the "required" class because of the empty() returns true.

I think that also any possible validation rule for the "others" field are not considered without this patch, but I'm not really sure.

@markstory
Owner

Does this pass the existing tests? It seems like this would change the return value to not return trailing digits. I know that's the point, but that is part of the current behaviour.

@markstory
Owner

Closing as this cannot be merged in anymore. If you'd like to update the changes so they can be merged in, we can take another look.

@markstory markstory closed this
@dereuromark dereuromark commented on the diff
lib/Cake/View/Helper.php
((7 lines not shown))
*
* @return string
*/
public function field() {
$entity = $this->entity();
- $count = count($entity);
- $last = $entity[$count - 1];
- if ($count > 2 && in_array($last, $this->_fieldSuffixes)) {
- $last = isset($entity[$count - 2]) ? $entity[$count - 2] : null;
+ for ($i = count($entity) - 1; $i >= 0; $i--) {
@dereuromark Collaborator

You should also extract the count() part from the foreach statement here into a separate variable (as it was before).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 6 additions and 6 deletions.
  1. +6 −6 lib/Cake/View/Helper.php
View
12 lib/Cake/View/Helper.php
@@ -550,17 +550,17 @@ public function model() {
/**
* Gets the currently-used model field of the rendering context.
- * Strips off field suffixes such as year, month, day, hour, min, meridian
- * when the current entity is longer than 2 elements.
+ * Strips off field suffixes such as year, month, day, hour, min, meridian and numbers.
*
* @return string
*/
public function field() {
$entity = $this->entity();
- $count = count($entity);
- $last = $entity[$count - 1];
- if ($count > 2 && in_array($last, $this->_fieldSuffixes)) {
- $last = isset($entity[$count - 2]) ? $entity[$count - 2] : null;
+ for ($i = count($entity) - 1; $i >= 0; $i--) {
@dereuromark Collaborator

You should also extract the count() part from the foreach statement here into a separate variable (as it was before).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ if (!is_numeric($entity[$i]) && !in_array($entity[$i], $this->_fieldSuffixes)) {
+ $last = $entity[$i];
+ break;
+ }
}
return $last;
}
Something went wrong with that request. Please try again.