From a633f81b966388385c6b033cff109b9c07f0a44a Mon Sep 17 00:00:00 2001 From: Franco Springveldt Date: Fri, 8 Sep 2017 13:57:33 +1200 Subject: [PATCH] FIX fixed failing tests --- README.md | 14 ++++---- src/Models/MultiForm.php | 33 +++++++++-------- src/Models/MultiFormSession.php | 9 ++--- src/Models/MultiFormStep.php | 6 ++-- src/extensions/MultiFormObjectDecorator.php | 8 ++--- tests/MultiFormObjectDecoratorTest.php | 2 +- tests/MultiFormTest.php | 40 +++++++++++++-------- tests/Stubs/MultiFormTestController.php | 13 +++---- 8 files changed, 69 insertions(+), 56 deletions(-) diff --git a/README.md b/README.md index ac552e8..67ab59f 100644 --- a/README.md +++ b/README.md @@ -158,7 +158,7 @@ a *FieldSet* with some form field objects. These are the fields that the form will render for the given step. Keep in mind that our multi-form also requires an end point. This step is the -final, and needs to have another variable set to let the multi-form system know +final one, and needs to have another variable set to let the multi-form system know this is the final step. So, if we assume that the last step in our process is @@ -220,7 +220,7 @@ class Page_Controller extends ContentController { } ``` -The `SurveyForm()` function will create a new instance our subclass of +The `SurveyForm()` function will create a new instance of our subclass of MultiForm, which in this example, is *SurveyForm*. This in turn will then set up all the form fields, actions, and validation available to each step, as well as the session. @@ -384,7 +384,7 @@ class SurveyForm extends MultiForm { parent::finish($data, $form); $steps = DataObject::get( - 'MultiFormStep', + MultiFormStep::class, "SessionID = {$this->session->ID}" ); @@ -480,21 +480,21 @@ MultiForm class: More than likely, you'll want the first one to be available when the form renders. To that effect, you can start placing templates in the -*templates/Includes* directory for your project. You need to call them the same +*templates/Includes* directory for your project. You need to name them the same as the class name for each step. For example, if you want *MembershipForm*, a subclass of *MultiFormStep* to have it's own template, you would put *MembershipForm.ss* into that directory, and run *?flush=1*. If you'd like a pre-existing template on how to customise the form step, have a -look at Form.ss that's found within the sapphire module. Use that template, as +look at Form.ss that's found within the framework module. Use that template, as a base for your new MembershipForm.ss template in your project templates. For more information on this, please [look at the Form documentation](http://doc.silverstripe.org/framework/en/topics/forms#custom-form-templates). ### getNextStep() -If you are wanting to override the next step (so, if you want the next step to -be something different based on a user's choice of input during the step, you +If you are wanting to override the next step (for example if you want the next step to +be something different based on a user's choice of input during the step) you can override getNextStep() on any given step to manually override what the next step should be. An example: diff --git a/src/Models/MultiForm.php b/src/Models/MultiForm.php index 1f42c90..7a7f44b 100644 --- a/src/Models/MultiForm.php +++ b/src/Models/MultiForm.php @@ -150,7 +150,7 @@ abstract class MultiForm extends Form if ($actionNames) { foreach ($actionNames as $exemptAction) { - if (!empty($_REQUEST[$exemptAction])) { + if (!empty($this->getRequest()->requestVar($exemptAction))) { $applyValidation = false; break; } @@ -160,14 +160,14 @@ abstract class MultiForm extends Form // Apply validation if the current step requires validation (is not exempt) if ($applyValidation) { if ($currentStep->getValidator()) { - $validator = $currentStep->getValidator(); + $this->setValidator($currentStep->getValidator()); } } // Give the fields, actions, and validation for the current step back to the parent Form class - parent::__construct($controller, $name, $fields, $actions, $validator); + parent::__construct($controller, $name, $fields, $actions); - $getVar = $this->config()->get_var; + $getVar = $this->getGetVar(); // Set a hidden field in our form with an encrypted hash to identify this session. $this->fields->push(HiddenField::create($getVar, false, $this->session->Hash)); @@ -201,7 +201,7 @@ abstract class MultiForm extends Form */ public function getGetVar() { - return $this->config()->get_var; + return $this->config()->get('get_var'); } /** @@ -231,7 +231,7 @@ abstract class MultiForm extends Form $StepID = $this->controller->getRequest()->getVar('StepID'); if (isset($StepID)) { $currentStep = DataObject::get_one( - 'MultiFormStep', + MultiFormStep::class, [ 'SessionID' => $this->session->ID, 'ID' => $StepID @@ -277,8 +277,11 @@ abstract class MultiForm extends Form * * @return MultiFormSession */ - public function getSession() + public function getMultiFormSession() { + if (!$this->session) { + $this->setSession(); + } return $this->session; } @@ -300,7 +303,7 @@ abstract class MultiForm extends Form // If there was no session found, create a new one instead if (!$this->session) { - $this->session = new MultiFormSession(); + $this->session = MultiFormSession::create(); } // Create encrypted identification to the session instance if it doesn't exist @@ -329,7 +332,7 @@ abstract class MultiForm extends Form public function getCurrentSession() { if (!$this->currentSessionHash) { - $this->currentSessionHash = $this->controller->getRequest()->getVar($this->config()->get_var); + $this->currentSessionHash = $this->controller->getRequest()->getVar($this->getGetVar()); if (!$this->currentSessionHash) { return false; @@ -357,7 +360,7 @@ abstract class MultiForm extends Form { $filter .= ($filter) ? ' AND ' : ''; $filter .= sprintf("\"SessionID\" = '%s'", $this->session->ID); - return DataObject::get('MultiFormStep', $filter); + return DataObject::get(MultiFormStep::class, $filter); } /** @@ -371,7 +374,7 @@ abstract class MultiForm extends Form public function getSavedStepByClass($className) { return DataObject::get_one( - 'MultiFormStep', + MultiFormStep::class, sprintf( "\"SessionID\" = '%s' AND \"ClassName\" = '%s'", $this->session->ID, @@ -476,7 +479,7 @@ abstract class MultiForm extends Form } if (!$this->getCurrentStep()->validateStep($data, $form)) { - Injector::inst()->get(Session::class)->set("FormInfo.{$form->FormName()}.data", $form->getData()); + $this->getRequest()->getSession()->set("FormInfo.{$form->FormName()}.data", $form->getData()); $this->controller->redirectBack(); return false; } @@ -511,7 +514,7 @@ abstract class MultiForm extends Form // built-in functionality). The data needs to be manually saved on error // so the form is re-populated. if (!$this->getCurrentStep()->validateStep($data, $form)) { - Injector::inst()->get(Session::class)->set("FormInfo.{$form->FormName()}.data", $form->getData()); + $this->getRequest()->getSession()->set("FormInfo.{$form->FormName()}.data", $form->getData()); $this->controller->redirectBack(); return false; } @@ -603,7 +606,7 @@ abstract class MultiForm extends Form { $action = parent::FormAction(); $action .= (strpos($action, '?')) ? '&' : '?'; - $action .= "{$this->config()->get_var}={$this->session->Hash}"; + $action .= "{$this->getGetVar()}={$this->session->Hash}"; return $action; } @@ -723,7 +726,7 @@ abstract class MultiForm extends Form */ public function getCompletedStepCount() { - $steps = DataObject::get('MultiFormStep', "\"SessionID\" = {$this->session->ID} && \"Data\" IS NOT NULL"); + $steps = DataObject::get(MultiFormStep::class, "\"SessionID\" = {$this->session->ID} && \"Data\" IS NOT NULL"); return $steps ? $steps->Count() : 0; } diff --git a/src/Models/MultiFormSession.php b/src/Models/MultiFormSession.php index 431a693..39ecb45 100644 --- a/src/Models/MultiFormSession.php +++ b/src/Models/MultiFormSession.php @@ -3,6 +3,7 @@ namespace SilverStripe\MultiForm\Models; use SilverStripe\ORM\DataObject; +use SilverStripe\Security\Member; use SilverStripe\Security\Security; /** @@ -23,12 +24,12 @@ class MultiFormSession extends DataObject ]; private static $has_one = [ - 'Submitter' => 'Member', - 'CurrentStep' => 'MultiFormStep' + 'Submitter' => Member::class, + 'CurrentStep' => MultiFormStep::class ]; private static $has_many = [ - 'FormSteps' => 'MultiFormStep' + 'FormSteps' => MultiFormStep::class ]; private static $table_name = 'MultiFormSession'; @@ -52,7 +53,7 @@ class MultiFormSession extends DataObject { // save submitter if a Member is logged in $currentMember = Security::getCurrentUser(); - if (!$this->SubmitterID && $currentMember->ID) { + if (!$this->SubmitterID && $currentMember) { $this->SubmitterID = $currentMember->ID; } diff --git a/src/Models/MultiFormStep.php b/src/Models/MultiFormStep.php index 1e03b21..ac52501 100644 --- a/src/Models/MultiFormStep.php +++ b/src/Models/MultiFormStep.php @@ -24,7 +24,7 @@ class MultiFormStep extends DataObject ]; private static $has_one = [ - 'Session' => 'MultiFormSession' + 'Session' => MultiFormSession::class ]; private static $table_name = 'MultiFormStep'; @@ -158,7 +158,7 @@ class MultiFormStep extends DataObject $form = $this->form; return Controller::join_links( $form->getDisplayLink(), - "?{$form->config()->get_var}={$this->getSession()->Hash}" + "?{$form->getGetVar()}={$this->getSession()->Hash}" ); } @@ -309,7 +309,7 @@ class MultiFormStep extends DataObject */ public function getPreviousStep() { - $steps = DataObject::get('MultiFormStep', "\"SessionID\" = {$this->SessionID}", '"LastEdited" DESC'); + $steps = DataObject::get(MultiFormStep::class, "\"SessionID\" = {$this->SessionID}", '"LastEdited" DESC'); if ($steps) { foreach ($steps as $step) { $step->setForm($this->form); diff --git a/src/extensions/MultiFormObjectDecorator.php b/src/extensions/MultiFormObjectDecorator.php index 73beb63..5ae6720 100644 --- a/src/extensions/MultiFormObjectDecorator.php +++ b/src/extensions/MultiFormObjectDecorator.php @@ -45,10 +45,10 @@ class MultiFormObjectDecorator extends DataExtension $query->addWhere("{$from[0]}.\"MultiFormIsTemporary\" = '0'"); return; } - - if (strpos($where[0], ".`ID` = ") === false - && strpos($where[0], ".ID = ") === false - && strpos($where[0], "ID = ") !== 0 + $filterKey = key($where[0]); + if (strpos($filterKey, ".`ID` = ") === false + && strpos($filterKey, ".ID = ") === false + && strpos($filterKey, "ID = ") !== 0 && !$this->wantsTemporary($query) ) { $from = array_values($query->getFrom()); diff --git a/tests/MultiFormObjectDecoratorTest.php b/tests/MultiFormObjectDecoratorTest.php index 9683f95..f0da517 100644 --- a/tests/MultiFormObjectDecoratorTest.php +++ b/tests/MultiFormObjectDecoratorTest.php @@ -13,7 +13,7 @@ class MultiFormObjectDecoratorTest extends SapphireTest MultiFormObjectDecoratorDataObject::class => [MultiFormObjectDecorator::class] ]; - protected $extraDataObjects = [ + protected static $extra_dataobjects = [ MultiFormObjectDecoratorDataObject::class ]; diff --git a/tests/MultiFormTest.php b/tests/MultiFormTest.php index 4430046..d0e9169 100644 --- a/tests/MultiFormTest.php +++ b/tests/MultiFormTest.php @@ -2,8 +2,13 @@ namespace SilverStripe\MultiForm\Tests; +use SilverStripe\Control\HTTPRequest; +use SilverStripe\Control\Session; use SilverStripe\Core\Config\Config; +use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\FunctionalTest; +use SilverStripe\MultiForm\Models\MultiForm; +use SilverStripe\MultiForm\Models\MultiFormSession; /** * MultiFormTest @@ -25,9 +30,11 @@ use SilverStripe\Dev\FunctionalTest; */ class MultiFormTest extends FunctionalTest { - public static $fixture_file = 'MultiFormTest.yml'; + /** + * @var MultiFormTestController + */ protected $controller; /** @@ -35,24 +42,32 @@ class MultiFormTest extends FunctionalTest */ protected $form; - public function setUp() + protected function setUp() { parent::setUp(); $this->controller = new MultiFormTestController(); - $this->form = $this->controller->Form(); + $this->controller->setRequest(new HTTPRequest('GET', '/')); + $this->controller->getRequest()->setSession(new Session([])); + $this->controller->pushCurrent(); + $form = $this->form = $this->controller->Form(); + Injector::inst()->registerService($form, MultiForm::class); + $this->form = $form; } public function testInitialisingForm() { $this->assertTrue(is_numeric($this->form->getCurrentStep()->ID) && ($this->form->getCurrentStep()->ID > 0)); - $this->assertTrue(is_numeric($this->form->getSession()->ID) && ($this->form->getSession()->ID > 0)); + $this->assertTrue( + is_numeric($this->form->getMultiFormSession()->ID) + && ($this->form->getMultiFormSession()->ID > 0) + ); $this->assertEquals(MultiFormTestStepOne::class, $this->form->getStartStep()); } public function testSessionGeneration() { - $this->assertTrue($this->form->getSession()->ID > 0); + $this->assertTrue($this->form->getMultiFormSession()->ID > 0); } public function testMemberLogging() @@ -62,7 +77,7 @@ class MultiFormTest extends FunctionalTest $userId = $this->logInWithPermission('ADMIN'); - $session = $this->form->getSession(); + $session = $this->form->getMultiFormSession(); $session->write(); $this->assertEquals($userId, $session->SubmitterID); @@ -86,9 +101,9 @@ class MultiFormTest extends FunctionalTest public function testCompletedSession() { - $this->form->setCurrentSessionHash($this->form->getSession()->Hash); - $this->assertInstanceOf('MultiFormSession', $this->form->getCurrentSession()); - $this->form->getSession()->markCompleted(); + $this->form->setCurrentSessionHash($this->form->getMultiFormSession()->Hash); + $this->assertInstanceOf(MultiFormSession::class, $this->form->getCurrentSession()); + $this->form->getMultiFormSession()->markCompleted(); $this->assertNull($this->form->getCurrentSession()); } @@ -97,13 +112,12 @@ class MultiFormTest extends FunctionalTest $this->form->setCurrentSessionHash('sdfsdf3432325325sfsdfdf'); // made up! // A new session is generated, even though we made up the identifier - $this->assertInstanceOf('MultiFormSession', $this->form->getSession()); + $this->assertInstanceOf(MultiFormSession::class, $this->form->getMultiFormSession()); } public function testCustomGetVar() { - Config::nest(); - Config::modify()->set('MultiForm', 'get_var', 'SuperSessionID'); + Config::modify()->set(MultiForm::class, 'get_var', 'SuperSessionID'); $form = $this->controller->Form(); $this->assertContains('SuperSessionID', $form::$ignored_fields, "GET var wasn't added to ignored fields"); @@ -117,7 +131,5 @@ class MultiFormTest extends FunctionalTest $form->getCurrentStep()->Link(), "Form step doesn't contain correct session ID parameter" ); - - Config::unnest(); } } diff --git a/tests/Stubs/MultiFormTestController.php b/tests/Stubs/MultiFormTestController.php index ee0a86b..651366e 100644 --- a/tests/Stubs/MultiFormTestController.php +++ b/tests/Stubs/MultiFormTestController.php @@ -3,6 +3,7 @@ namespace SilverStripe\MultiForm\Tests; use SilverStripe\Control\Controller; +use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\TestOnly; /** @@ -11,15 +12,11 @@ use SilverStripe\Dev\TestOnly; */ class MultiFormTestController extends Controller implements TestOnly { - public function Link($action = null) - { - return self::class; - } + private static $url_segment = 'MultiFormTestController'; - public function Form($request = null) + public function Form() { - $form = new MultiFormTestForm($this, 'Form'); - $form->setHTMLID(self::class); - return $form; + return Injector::inst()->get(MultiFormTestForm::class, false, [$this, 'Form']) + ->setHTMLID(MultiFormTestForm::class); } }