From 4728586365f58b4d98000942efa135c5dc0b31d8 Mon Sep 17 00:00:00 2001 From: Mark van Driel Date: Wed, 8 Mar 2017 13:15:00 +0100 Subject: [PATCH 1/7] Make error class of div in FormHelper::input configurable --- .../Test/Case/View/Helper/FormHelperTest.php | 66 +++++++++++++++++++ lib/Cake/View/Helper/FormHelper.php | 3 +- 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/lib/Cake/Test/Case/View/Helper/FormHelperTest.php b/lib/Cake/Test/Case/View/Helper/FormHelperTest.php index 7c9f70cc8..bea3fbe3c 100644 --- a/lib/Cake/Test/Case/View/Helper/FormHelperTest.php +++ b/lib/Cake/Test/Case/View/Helper/FormHelperTest.php @@ -10734,4 +10734,70 @@ class FormHelperTest extends CakeTestCase { $this->assertAttributeEquals($here, '_lastAction', $this->Form, "_lastAction shouldn't be empty."); } +/** + * Tests the 'errorClass' option when error is returned. + * + * @return void + */ + public function testInputErrorClass() { + $result = $this->Form->input('Contact.field', array( + 'type' => 'text', + 'div' => array('errorClass' => 'has-error'), + 'error' => true + )); + $expected = array( + 'div' => array('class' => 'input text has-error'), + 'label' => array('for' => 'ContactField'), + 'Field', + '/label', + 'input' => array( + 'type' => 'text', 'name' => 'data[Contact][field]', + 'id' => 'ContactField', 'class' => 'form-error' + ), + array('div' => array('class' => 'error')), + 'Badness!', + '/div' + ); + $this->assertTags($result, $expected); + + $result = $this->Form->input('Contact.second', array( + 'type' => 'text', + 'error' => array('attributes' => array('class' => 'error')) + )); + $expected = array( + 'div' => array('class' => 'input text error'), + 'label' => array('for' => 'ContactSecond'), + 'Field', + '/label', + 'input' => array( + 'type' => 'text', 'name' => 'data[Contact][second]', + 'id' => 'ContactSecond', 'class' => 'form-error' + ), + array('div' => array('class' => 'error')), + 'Badness!', + '/div' + ); + $this->assertTags($result, $expected); + + $result = $this->Form->input('Contact.third', array( + 'type' => 'text', + 'div' => array('errorClass' => 'has-error'), + 'error' => array('attributes' => array('class' => 'form-control-feedback')) + )); + $expected = array( + 'div' => array('class' => 'input text has-error'), + 'label' => array('for' => 'ContactThird'), + 'Field', + '/label', + 'input' => array( + 'type' => 'text', 'name' => 'data[Contact][third]', + 'id' => 'ContactThird', 'class' => 'form-error' + ), + array('div' => array('class' => 'form-control-feedback')), + 'Badness!', + '/div' + ); + $this->assertTags($result, $expected); + } + } diff --git a/lib/Cake/View/Helper/FormHelper.php b/lib/Cake/View/Helper/FormHelper.php index 0d6dd7dd5..6e5a56482 100644 --- a/lib/Cake/View/Helper/FormHelper.php +++ b/lib/Cake/View/Helper/FormHelper.php @@ -1068,7 +1068,7 @@ class FormHelper extends AppHelper { if ($type !== 'hidden' && $error !== false) { $errMsg = $this->error($fieldName, $error); if ($errMsg) { - $divOptions = $this->addClass($divOptions, 'error'); + $divOptions = $this->addClass($divOptions, Hash::get($divOptions, 'errorClass', 'error')); if ($errorMessage) { $out['error'] = $errMsg; } @@ -1089,6 +1089,7 @@ class FormHelper extends AppHelper { if (!empty($divOptions['tag'])) { $tag = $divOptions['tag']; unset($divOptions['tag']); + unset($divOptions['errorClass']); $output = $this->Html->tag($tag, $output, $divOptions); } return $output; From 708e960968020f5df73ffb6ade7047c0e1eb4ad9 Mon Sep 17 00:00:00 2001 From: Mark van Driel Date: Wed, 8 Mar 2017 13:43:40 +0100 Subject: [PATCH 2/7] Fixed tests --- .../Test/Case/View/Helper/FormHelperTest.php | 52 ++++++++++--------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/lib/Cake/Test/Case/View/Helper/FormHelperTest.php b/lib/Cake/Test/Case/View/Helper/FormHelperTest.php index bea3fbe3c..33410f68b 100644 --- a/lib/Cake/Test/Case/View/Helper/FormHelperTest.php +++ b/lib/Cake/Test/Case/View/Helper/FormHelperTest.php @@ -10740,10 +10740,12 @@ class FormHelperTest extends CakeTestCase { * @return void */ public function testInputErrorClass() { + $Contact = ClassRegistry::getObject('Contact'); + $Contact->validationErrors['field'] = array('Badness!'); + $result = $this->Form->input('Contact.field', array( 'type' => 'text', - 'div' => array('errorClass' => 'has-error'), - 'error' => true + 'div' => array('errorClass' => 'has-error') )); $expected = array( 'div' => array('class' => 'input text has-error'), @@ -10754,44 +10756,44 @@ class FormHelperTest extends CakeTestCase { 'type' => 'text', 'name' => 'data[Contact][field]', 'id' => 'ContactField', 'class' => 'form-error' ), + array('div' => array('class' => 'error-message')), + 'Badness!', + '/div' + ); + $this->assertTags($result, $expected); + + $result = $this->Form->input('Contact.field', array( + 'type' => 'text', + 'error' => array('attributes' => array('class' => 'error')) + )); + $expected = array( + 'div' => array('class' => 'input text error'), + 'label' => array('for' => 'ContactField'), + 'Field', + '/label', + 'input' => array( + 'type' => 'text', 'name' => 'data[Contact][field]', + 'id' => 'ContactField', 'class' => 'form-error' + ), array('div' => array('class' => 'error')), 'Badness!', '/div' ); $this->assertTags($result, $expected); - $result = $this->Form->input('Contact.second', array( - 'type' => 'text', - 'error' => array('attributes' => array('class' => 'error')) - )); - $expected = array( - 'div' => array('class' => 'input text error'), - 'label' => array('for' => 'ContactSecond'), - 'Field', - '/label', - 'input' => array( - 'type' => 'text', 'name' => 'data[Contact][second]', - 'id' => 'ContactSecond', 'class' => 'form-error' - ), - array('div' => array('class' => 'error')), - 'Badness!', - '/div' - ); - $this->assertTags($result, $expected); - - $result = $this->Form->input('Contact.third', array( + $result = $this->Form->input('Contact.field', array( 'type' => 'text', 'div' => array('errorClass' => 'has-error'), 'error' => array('attributes' => array('class' => 'form-control-feedback')) )); $expected = array( 'div' => array('class' => 'input text has-error'), - 'label' => array('for' => 'ContactThird'), + 'label' => array('for' => 'ContactField'), 'Field', '/label', 'input' => array( - 'type' => 'text', 'name' => 'data[Contact][third]', - 'id' => 'ContactThird', 'class' => 'form-error' + 'type' => 'text', 'name' => 'data[Contact][field]', + 'id' => 'ContactField', 'class' => 'form-error' ), array('div' => array('class' => 'form-control-feedback')), 'Badness!', From c8aefeb4951029ade45dff02fcda950dee326ad5 Mon Sep 17 00:00:00 2001 From: Mark van Driel Date: Wed, 8 Mar 2017 14:16:08 +0100 Subject: [PATCH 3/7] Cs fixes --- lib/Cake/Test/Case/View/Helper/FormHelperTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Cake/Test/Case/View/Helper/FormHelperTest.php b/lib/Cake/Test/Case/View/Helper/FormHelperTest.php index 33410f68b..2fe621ff8 100644 --- a/lib/Cake/Test/Case/View/Helper/FormHelperTest.php +++ b/lib/Cake/Test/Case/View/Helper/FormHelperTest.php @@ -10781,7 +10781,7 @@ class FormHelperTest extends CakeTestCase { ); $this->assertTags($result, $expected); - $result = $this->Form->input('Contact.field', array( + $result = $this->Form->input('Contact.field', array( 'type' => 'text', 'div' => array('errorClass' => 'has-error'), 'error' => array('attributes' => array('class' => 'form-control-feedback')) From 25d597910f7f868e2c483ac13e1007990c793847 Mon Sep 17 00:00:00 2001 From: Mark van Driel Date: Wed, 8 Mar 2017 23:19:37 +0100 Subject: [PATCH 4/7] Code cleanup --- lib/Cake/View/Helper/FormHelper.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/Cake/View/Helper/FormHelper.php b/lib/Cake/View/Helper/FormHelper.php index 6e5a56482..810d5a358 100644 --- a/lib/Cake/View/Helper/FormHelper.php +++ b/lib/Cake/View/Helper/FormHelper.php @@ -1088,8 +1088,7 @@ class FormHelper extends AppHelper { if (!empty($divOptions['tag'])) { $tag = $divOptions['tag']; - unset($divOptions['tag']); - unset($divOptions['errorClass']); + unset($divOptions['tag'], $divOptions['errorClass']); $output = $this->Html->tag($tag, $output, $divOptions); } return $output; From 9dbeeaa1fa9e23042192eb1a033b526f4fd69003 Mon Sep 17 00:00:00 2001 From: mark_story Date: Wed, 15 Mar 2017 15:58:54 -0400 Subject: [PATCH 5/7] Add test for #10418 Show that find(list) does not drop keys with values = 0. Refs #10418 --- lib/Cake/Test/Case/Model/ModelReadTest.php | 27 ++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/lib/Cake/Test/Case/Model/ModelReadTest.php b/lib/Cake/Test/Case/Model/ModelReadTest.php index 7723a8cfc..e08acb173 100644 --- a/lib/Cake/Test/Case/Model/ModelReadTest.php +++ b/lib/Cake/Test/Case/Model/ModelReadTest.php @@ -6960,6 +6960,33 @@ class ModelReadTest extends BaseModelTest { $this->assertEquals($expected, $result); } +/** + * test find('list') method + * + * @return void + */ + public function testFindListZeroId() { + $this->loadFixtures('Article'); + + $model = new Article(); + $model->displayField = 'title'; + $model->save(array( + 'title' => 'Zeroth Article', + 'user_id' => 0, + 'published' => 'Y' + )); + + $result = $model->find('list', array( + 'fields' => array('user_id', 'title') + )); + $expected = array( + 0 => 'Zeroth Article', + 1 => 'Third Article', + 3 => 'Second Article' + ); + $this->assertEquals($expected, $result); + } + /** * Test that find(list) works with array conditions that have only one element. * From c5e31e590df0673d60e931932abd4fcf28f8134a Mon Sep 17 00:00:00 2001 From: mark_story Date: Wed, 15 Mar 2017 21:33:19 -0400 Subject: [PATCH 6/7] Revise test case based on feedback in #10418 Try and change the test around to trigger the issue. I'm still not able to reproduce the issue. --- lib/Cake/Test/Case/Model/ModelReadTest.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/Cake/Test/Case/Model/ModelReadTest.php b/lib/Cake/Test/Case/Model/ModelReadTest.php index e08acb173..48ca338a5 100644 --- a/lib/Cake/Test/Case/Model/ModelReadTest.php +++ b/lib/Cake/Test/Case/Model/ModelReadTest.php @@ -6965,7 +6965,7 @@ class ModelReadTest extends BaseModelTest { * * @return void */ - public function testFindListZeroId() { + public function testFindListZeroValue() { $this->loadFixtures('Article'); $model = new Article(); @@ -6977,12 +6977,13 @@ class ModelReadTest extends BaseModelTest { )); $result = $model->find('list', array( - 'fields' => array('user_id', 'title') + 'fields' => array('title', 'user_id') )); $expected = array( - 0 => 'Zeroth Article', - 1 => 'Third Article', - 3 => 'Second Article' + 'Zeroth Article' => 0, + 'First Article' => 1, + 'Second Article' => 3, + 'Third Article' => 1, ); $this->assertEquals($expected, $result); } From ccc90066204229df1e8088de4ca1266267151397 Mon Sep 17 00:00:00 2001 From: mark_story Date: Thu, 16 Mar 2017 11:31:20 -0400 Subject: [PATCH 7/7] Unset the active user data on logout. When using stateless authentication the current user should be cleared after logout to maintain consistency with session based authentication. Refs #10422 --- lib/Cake/Controller/Component/AuthComponent.php | 1 + .../Controller/Component/AuthComponentTest.php | 17 +++++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/lib/Cake/Controller/Component/AuthComponent.php b/lib/Cake/Controller/Component/AuthComponent.php index 54e7034ab..de14b5c07 100644 --- a/lib/Cake/Controller/Component/AuthComponent.php +++ b/lib/Cake/Controller/Component/AuthComponent.php @@ -645,6 +645,7 @@ class AuthComponent extends Component { foreach ($this->_authenticateObjects as $auth) { $auth->logout($user); } + static::$_user = array(); $this->Session->delete(static::$sessionKey); $this->Session->delete('Auth.redirect'); $this->Session->renew(); diff --git a/lib/Cake/Test/Case/Controller/Component/AuthComponentTest.php b/lib/Cake/Test/Case/Controller/Component/AuthComponentTest.php index 5da15d1e9..83202b65b 100644 --- a/lib/Cake/Test/Case/Controller/Component/AuthComponentTest.php +++ b/lib/Cake/Test/Case/Controller/Component/AuthComponentTest.php @@ -1428,6 +1428,23 @@ class AuthComponentTest extends CakeTestCase { $this->assertNull($this->Auth->Session->read('Auth.redirect')); } +/** + * test that logout removes the active user data as well for stateless auth + * + * @return void + */ + public function testLogoutRemoveUser() { + $oldKey = AuthComponent::$sessionKey; + AuthComponent::$sessionKey = false; + $this->Auth->login(array('id' => 1, 'username' => 'mariano')); + $this->assertSame('mariano', $this->Auth->user('username')); + + $this->Auth->logout(); + AuthComponent::$sessionKey = $oldKey; + + $this->assertNull($this->Auth->user('username')); + } + /** * Logout should trigger a logout method on authentication objects. *