Skip to content

Some refactoring in order to reduce the number of redirects #23

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bancer wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Some refactoring in order to reduce the number of redirects #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
faf6af1
#more unit tests
bancer Mar 8, 2017
9b7c204
adjusted unit tests
bancer Mar 8, 2017
15ca341
better clear session in tests
bancer Mar 8, 2017
e1966d7
unit tests adjusted
bancer Mar 8, 2017
c21d679
unit tests
bancer Mar 8, 2017
e65b739
unit tests improved
bancer Mar 8, 2017
9fdef2e
Some refactoring in order to reduce the number of redirects
bancer Mar 8, 2017
0fb4e4e
more refactoring
bancer Mar 8, 2017
cebb126
more docs and reset the expected step in branch()
bancer Mar 8, 2017
29e05ca
updates expected step after save() and updates steps after unbranch() is
bancer Mar 8, 2017
d36fea2
ensure the current step is set properly
bancer Mar 8, 2017
58f7f6a
Merge branch 'master' of https://github.com/ProLoser/CakePHP-Wizard into
bancer Mar 8, 2017
b348094
Fixed indefinite loop bug.
bancer Mar 9, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions Controller/Component/WizardComponent.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,13 @@ class WizardComponent extends Component {
*/
protected $_wizardUrl = array();

/**
* Holds the array with steps and branches from the initial Wizard configuration.
*
* @var array
*/
protected $_stepsAndBranches = array();

/**
* Initializes WizardComponent for use in the controller
*
Expand All @@ -216,6 +223,7 @@ class WizardComponent extends Component {
public function initialize(Controller $controller) {
$this->controller = $controller;
$this->__setSessionKeys();
$this->_stepsAndBranches = $this->steps;
}

/**
Expand Down Expand Up @@ -244,16 +252,27 @@ private function __setSessionKeys() {
*/
public function startup(Controller $controller) {
$this->__setSessionKeys();
$this->steps = $this->_parseSteps($this->steps);
$this->config('action', $this->action);
$this->config('steps', $this->steps);
$this->_configSteps($this->steps);
if (!in_array('Wizard.Wizard', $this->controller->helpers) && !array_key_exists('Wizard.Wizard', $this->controller->helpers)) {
$this->controller->helpers['Wizard.Wizard'] = array(
'sessionRootKey' => $this->sessionRootKey,
);
}
}

/**
* Parses the steps array by stripping off nested arrays not included in the branches
* and writes a simple array with the correct steps to session.
*
* @param array $steps Array to be parsed for nested arrays.
* @return void
*/
protected function _configSteps($steps) {
$this->steps = $this->_parseSteps($steps);
$this->config('steps', $this->steps);
}

/**
* Parses the steps array by stripping off nested arrays not included in the branches
* and returns a simple array with the correct steps.
Expand Down Expand Up @@ -510,6 +529,9 @@ protected function _validStep($step) {
* @return void
*/
protected function _setCurrentStep($step) {
if (!in_array($step, $this->steps)) {
return;
}
Copy link
Owner

@ProLoser ProLoser Mar 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of checks like this. I mean I'd be concerned where they got the step from if it's an invalid choice, this is sort of an error you should never reach.

Copy link
Collaborator Author

@bancer bancer Mar 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not remember exactly why that was needed. I think that has something to do with internal workings of this component to prevent setting the future step that is not in $this->steps yet. An infinite loop happened without this check. Notice while (current($this->steps) != $step) below. The clients cannot call _setCurrentStep() directly as it is protected so there is no need to worry.

Copy link
Owner

@ProLoser ProLoser Mar 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be the exact reason to leave comments in the code. Otherwise I would take out the code rather than just hiding it under private.

$this->_currentStep = reset($this->steps);
while (current($this->steps) != $step) {
$this->_currentStep = next($this->steps);
Expand Down Expand Up @@ -556,6 +578,8 @@ public function save($step = null, $data = null) {
$data = $this->controller->request->data;
}
$this->controller->Session->write("$this->_sessionKey.$step", $data);
$this->_getExpectedStep();
$this->_setCurrentStep($step);
}

/**
Expand Down Expand Up @@ -603,6 +627,7 @@ public function branch($name, $skip = false) {
}
$branches[$name] = $value;
$this->controller->Session->write($this->_branchKey, $branches);
$this->_configSteps($this->_stepsAndBranches);
}

/**
Expand Down Expand Up @@ -674,6 +699,7 @@ public function delete($key = null) {
*/
public function unbranch($branch) {
$this->controller->Session->delete("$this->_branchKey.$branch");
$this->_configSteps($this->_stepsAndBranches);
}

}
100 changes: 99 additions & 1 deletion Test/Case/Controller/Component/WizardComponentTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class WizardUserMock extends Model {
/**
* AuthTestController class
*
* @property WizardComponent $Wizard
* @package Wizard.Test.Case.Controller.Component
*/
class WizardTestController extends Controller {
Expand Down Expand Up @@ -71,6 +72,22 @@ public function processStep2() {
return false;
}

public function processGender() {
if (!empty($this->request->data)) {
if ($this->Wizard->defaultBranch === false) {
if ($this->request->data['WizardUserMock']['gender'] == 'female') {
$this->Wizard->unbranch('male');
$this->Wizard->branch('female');
} else {
$this->Wizard->unbranch('female');
$this->Wizard->branch('male');
}
}
return true;
}
return false;
}

public function processStep3() {
if (!empty($this->request->data)) {
return true;
Expand Down Expand Up @@ -136,7 +153,7 @@ public function setUp() {
*/
public function tearDown() {
parent::tearDown();
$this->Wizard->Session->delete('Wizard');
CakeSession::destroy();
unset($this->Controller, $this->Wizard);
}

Expand Down Expand Up @@ -374,6 +391,87 @@ public function testProcessStepOnePost() {
$this->assertEquals($expectedSession, $resultSession);
}

/**
* Tests 'autoAdvance' and 'defaultBranch' settings set to false and manual call to `branch()`.
*
* @return void
*/
public function testProcessGenderPost() {
$this->Wizard->Session->delete('Wizard');
unset($this->Controller, $this->Wizard);
$CakeRequest = new CakeRequest(null, false);
$CakeResponse = $this->getMock('CakeResponse', array('send'));
$this->Controller = new WizardTestController($CakeRequest, $CakeResponse);
$this->Controller->components['Wizard.Wizard']['autoAdvance'] = false;
$this->Controller->components['Wizard.Wizard']['defaultBranch'] = false;
$ComponentCollection = new ComponentCollection();
$ComponentCollection->init($this->Controller);
$this->Controller->Components->init($this->Controller);
$this->Wizard = $this->Controller->Wizard;
$this->Wizard->initialize($this->Controller);

// Set session prerequisites.
$session = array(
'config' => array(
'steps' => array(
'step1',
'step2',
'gender',
'confirmation',
),
'action' => 'wizard',
'expectedStep' => 'gender',
'activeStep' => 'gender',
),
'WizardTest' => array(
'step1' => array(),
'step2' => array(),
),
);
$this->Wizard->Session->write('Wizard', $session);

$this->Wizard->startup($this->Controller);
$postData = array(
'WizardUserMock' => array(
'gender' => 'female',
),
);
$this->Wizard->controller->request->data = $postData;
$CakeResponse = $this->Wizard->process('gender');

$expectedSession = array(
'branches' => array(
'WizardTest' => array(
'female' => 'branch',
),
),
'config' => array(
'steps' => array(
'step1',
'step2',
'gender',
'step4',
'step5',
'confirmation',
),
'action' => 'wizard',
'expectedStep' => 'step4',
'activeStep' => 'gender',
),
'WizardTest' => array(
'step1' => array(),
'step2' => array(),
'gender' => $postData,
),
);
$resultSession = $this->Wizard->Session->read('Wizard');
$this->assertEquals($expectedSession, $resultSession);

$this->assertInstanceOf('CakeResponse', $CakeResponse);
$headers = $CakeResponse->header();
$this->assertContains('/wizard/step4', $headers['Location']);
}

public function testProcessAutovalidatePost() {
// Set session prerequisites.
$session = array(
Expand Down