Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
110 changes: 57 additions & 53 deletions core/Form/Form.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,15 @@
**/
final class Form extends RegulatedForm
{
const WRONG = 0x0001;
const MISSING = 0x0002;

private $errors = array();
/**
* @deprecated
*/
const WRONG = BasePrimitive::WRONG;
/**
* @deprecated
*/
const MISSING = BasePrimitive::MISSING;

private $labels = array();
private $describedLabels = array();

Expand All @@ -40,22 +45,27 @@ public static function create()

public function getErrors()
{
return array_merge($this->errors, $this->violated);
$errors = array();

foreach ($this->primitives as $name => $prm)
if ($error = $prm->getError())
$errors[$name] = $error;

return array_merge($errors, $this->violated);
}

public function hasError($name)
{
return array_key_exists($name, $this->errors)
|| array_key_exists($name, $this->violated);
return array_key_exists($name, $this->getErrors());
}

public function getError($name)
{
if (array_key_exists($name, $this->errors)) {
return $this->errors[$name];
} elseif (array_key_exists($name, $this->violated)) {
return $this->violated[$name];
}
$errors = $this->getErrors();

if (array_key_exists($name, $errors))
return $errors[$name];

return null;
Copy link
Contributor

Choose a reason for hiding this comment

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

правильно. ООП-шно. но hasError не имеет, более, смысла в контексте этих изменений, ибо если err !== null вернуть err иначе вернуть null =)

}

Expand Down Expand Up @@ -87,7 +97,9 @@ public function getInnerErrors()
**/
public function dropAllErrors()
{
$this->errors = array();
foreach ($this->primitives as $prm)
$prm->dropError();

$this->violated = array();

return $this;
Expand Down Expand Up @@ -122,7 +134,7 @@ public function disableImportFiltering()
**/
public function markMissing($primitiveName)
{
return $this->markCustom($primitiveName, Form::MISSING);
return $this->markCustom($primitiveName, BasePrimitive::MISSING);
}

/**
Expand All @@ -132,25 +144,16 @@ public function markMissing($primitiveName)
**/
public function markWrong($name)
{
if (isset($this->primitives[$name]))
$this->errors[$name] = self::WRONG;
elseif (isset($this->rules[$name]))
$this->violated[$name] = self::WRONG;
else
throw new MissingElementException(
$name.' does not match known primitives or rules'
);

return $this;
return $this->markCustom($name, BasePrimitive::WRONG);
}

/**
* @return Form
**/
Copy link
Contributor

Choose a reason for hiding this comment

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

Может быть заодно тогда привести к одному виду аргументы методов класса? $primitiveName или $name ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Согласен!

public function markGood($primitiveName)
{
if (isset($this->primitives[$primitiveName]))
unset($this->errors[$primitiveName]);
if($this->exists($primitiveName))
$this->get($primitiveName)->dropError();
elseif (isset($this->rules[$primitiveName]))
unset($this->violated[$primitiveName]);
else
Expand All @@ -166,11 +169,16 @@ public function markGood($primitiveName)
*
* @return Form
**/
public function markCustom($primitiveName, $customMark)
public function markCustom($name, $customMark)
{
Assert::isInteger($customMark);

$this->errors[$this->get($primitiveName)->getName()] = $customMark;
if ($this->exists($name))
$this->get($name)->setError($customMark);
elseif (isset($this->rules[$name]))
$this->violated[$name] = $customMark;
else
throw new MissingElementException(
$name.' does not match known primitives or rules'
);

return $this;
}
Expand Down Expand Up @@ -201,12 +209,10 @@ public function getTextualErrorFor($name)
)
return $this->labels[$name][$this->violated[$name]];
elseif (
isset(
$this->errors[$name],
$this->labels[$name][$this->errors[$name]]
)
($error = $this->getError($name) )
&& isset($this->labels[$name][$error])
)
return $this->labels[$name][$this->errors[$name]];
return $this->labels[$name][$error];
else
return null;
}
Expand All @@ -221,12 +227,10 @@ public function getErrorDescriptionFor($name)
)
return $this->describedLabels[$name][$this->violated[$name]];
elseif (
isset(
$this->errors[$name],
$this->describedLabels[$name][$this->errors[$name]]
)
($error = $this->getError($name) )
&& isset($this->describedLabels[$name][$error])
)
return $this->describedLabels[$name][$this->errors[$name]];
return $this->describedLabels[$name][$error];
else
return null;
}
Expand Down Expand Up @@ -421,27 +425,27 @@ private function importPrimitive($scope, BasePrimitive $prm)
**/
private function checkImportResult(BasePrimitive $prm, $result)
{
if (
$name = $prm->getName();
$error = $prm->getError();

if(
$prm instanceof PrimitiveAlias
&& $result !== null
&& !($result === null)
)
$this->markGood($prm->getInner()->getName());

$name = $prm->getName();


if (null === $result) {
if ($prm->isRequired())
$this->errors[$name] = self::MISSING;
$this->markCustom($name, BasePrimitive::MISSING);
Copy link
Member

Choose a reason for hiding this comment

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

по описанной идее за MISSING должен отвечать примитив а не Form

Copy link
Member Author

Choose a reason for hiding this comment

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

Это первая итерация, по хорошему все проверки должны проходить в примитивах ну собсно он должен выстовлять себе ошибки если таковы были.

Но я тут начал эту тему делать и прифигел, тама все примитивы надо будет затронуть практически, по этому оставляем на потом.


} elseif (true === $result) {
unset($this->errors[$name]);

} elseif ($error = $prm->getCustomError()) {
$this->markGood($name);

$this->errors[$name] = $error;

} else
$this->errors[$name] = self::WRONG;
} elseif ($error) {
$this->markCustom($name, $error);

} else
$this->markCustom($name, BasePrimitive::WRONG);

return $this;
}
Expand Down
51 changes: 46 additions & 5 deletions core/Form/Primitives/BasePrimitive.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
**/
abstract class BasePrimitive
{
const WRONG = 0x0001;
const MISSING = 0x0002;

protected $name = null;
protected $default = null;
protected $value = null;
Expand All @@ -25,8 +28,13 @@ abstract class BasePrimitive
protected $imported = false;

protected $raw = null;

protected $customError = null;

protected $error = null;

/**
* @deprecated by error
*/
protected $customError = null;

public function __construct($name)
{
Expand Down Expand Up @@ -171,6 +179,7 @@ public function clean()
$this->raw = null;
$this->value = null;
$this->imported = false;
$this->dropError();

return $this;
}
Expand All @@ -184,13 +193,45 @@ public function exportValue()
{
return $this->value;
}


public function getError()
{
return $this->error | $this->customError;
}

/**
* @return BasePrimitive
**/
public function setError($error)
{
Assert::isPositiveInteger($error);

$this->error = $error;
$this->customError = $error;

return $this;
}

/**
* @return BasePrimitive
**/
public function dropError()
{
$this->error = null;
$this->customError = null;

return $this;
}

/**
* @deprecated by getError
*/
public function getCustomError()
{
return $this->customError;
return $this->getError();
}

protected function import($scope)
public function import($scope)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ура )

{
if (
!empty($scope[$this->name])
Expand Down
15 changes: 10 additions & 5 deletions core/Form/Primitives/PrimitiveAlias.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -144,18 +144,23 @@ public function exportValue()
{
return $this->primitive->exportValue();
}

public function getCustomError()

public function getError()
{
return $this->primitive->getError();
}

public function setError($error)
{
return $this->primitive->getCustomError();
return $this->primitive->setError($error);
}

public function import($scope)
{
if (array_key_exists($this->name, $scope))
return
$this->primitive->import(
array($this->primitive->getName() => $scope[$this->name])
$this->primitive->importValue(
$scope[$this->name]
);
Copy link
Contributor

Choose a reason for hiding this comment

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

А для этого кейса у нас есть тест? Кажется могло сломаться.

Copy link
Member Author

Choose a reason for hiding this comment

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

Все старые тесты работают :-)


return null;
Expand Down
16 changes: 8 additions & 8 deletions test/core/FormPrimitivesDateTest.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -123,25 +123,25 @@ protected function processInvalidBy($scope, $data)
import($scope);

$this->assertEquals(
$form->getValue('test'),
null
null,
$form->getValue('test')
);

$this->assertEquals(
$form->getRawValue('test'),
$data
$data,
$form->getRawValue('test')
);

$this->assertEquals(
$form->get('test')->isImported(),
true
true,
$form->get('test')->isImported()
);

$this->assertEquals(
$form->getErrors(),
array(
'test' => Form::WRONG,
)
),
$form->getErrors()
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

что не правильно в том что было?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ожидаетя, ТоЧтоЕсть - правильно
Так ожидает аргументы assertEquals а было наоборот!

Copy link
Member

Choose a reason for hiding this comment

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

phpunit позиционирует в таких ассертах первый аргумент как ожидаемый (тот который должен быть), второй аргумент как тот, который получился в результате выполнения теста.
В onPHP по какой-то причине часть тестов (большая, пожалуй) написана с перепутанным порядко аргументов, часть с верным.
Думаю стоит все же двигаться в направлении использования правильного порядка аргументов. Впрочем если происходит какая-то ошибка, то все равно нужно залазить и смотреть что случилось.

Copy link
Contributor

Choose a reason for hiding this comment

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

Отлично )


Expand Down