Skip to content

Improve error message #16

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
BertrandF23 wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Improve error message #16

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6d967d3
Explicit error message in the Mapper for keyExists function
BertrandF23 Oct 27, 2017
86b2b3b
Unify parameter name for no_results instead of noResults
BertrandF23 Oct 27, 2017
6a992da
Merge branch 'master' of https://github.com/smartboxgroup/integration…
BertrandF23 Nov 6, 2017
8a0014e
Remove changes in ConfigurableServiceHelper.php after split of the Pu…
BertrandF23 Nov 6, 2017
22f03a9
Add Tests for mapper keyExists
BertrandF23 Nov 6, 2017
eca24f3
Added tests in th Mapper for keyExists exceptions
BertrandF23 Nov 6, 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
14 changes: 8 additions & 6 deletions Configurability/ConfigurableServiceHelper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class ConfigurableServiceHelper
const CONF_MESSAGE = 'message';
const CONF_RECOVERABLE = 'recoverable';

const CONF_NO_RESULTS = 'noResults';
const CONF_NO_RESULTS = 'no_results';
Copy link
Copy Markdown
Contributor

@Thevart Thevart Nov 1, 2017

Choose a reason for hiding this comment

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

Hi @BertrandF23 ,

This is potentially a breaking change on the framework for other user of the bundle.

I would recommend not to do it as part of this pull request.

Copy link
Copy Markdown
Contributor Author

@BertrandF23 BertrandF23 Nov 6, 2017

Choose a reason for hiding this comment

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

Ok, was split and put in another pull request :-).

const STEP_DEFINE = 'define';

const STEP_VALIDATE = 'validate';
Expand Down Expand Up @@ -96,11 +96,13 @@ public function createContext(array $options, MessageInterface $message = null,
return $context;
}

public function resolveArray($input, &$context){
public function resolveArray($input, &$context)
{
$output = [];
foreach($input as $key => $value){
$output[$key] = $this->resolve($value,$context);
foreach ($input as $key => $value) {
$output[$key] = $this->resolve($value, $context);
}

return $output;
}

Expand Down Expand Up @@ -163,6 +165,7 @@ public function executeStep($stepAction, &$stepActionParams, &$options, array &$
break;
case self::STEP_VALIDATE:
$this->validate($stepActionParams, $context);

return true;
break;
default:
Expand Down Expand Up @@ -196,7 +199,6 @@ public function define(array $definitions, array &$context)

public function validate(array $stepConfig, array &$context)
{

$config = $this->validateResolver->resolve($stepConfig);

$rule = $config[self::CONF_RULE];
Expand All @@ -206,7 +208,7 @@ public function validate(array $stepConfig, array &$context)
$display_message = $config[self::CONF_DISPLAY_MESSAGE];

$evaluation = $this->resolve($rule, $context);
if ($evaluation !== true) {
if (true !== $evaluation) {
$message = $this->resolve($message, $context);
if ($no_results) {
throw new NoResultsException($message);
Expand Down
15 changes: 10 additions & 5 deletions Tools/Mapper/Mapper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public function addMappings(array $mappings)
*/
public function formatDate($format, \DateTime $date = null)
{
if ($date === null) {
if (null === $date) {
return null;
}

Expand Down Expand Up @@ -87,7 +87,7 @@ public function resolve(&$mapping, &$obj, &$context)
foreach ($mapping as $key => $value) {
$resolved = $this->resolve($value, $obj, $context);

if ($resolved !== null) {
if (null !== $resolved) {
$res[$key] = $resolved;
}
}
Expand Down Expand Up @@ -146,6 +146,10 @@ public function mapAll($elements, $mappingName, $context = [])
*/
public function keyExists(array $obj, $key)
{
if (!is_string($key) and !is_integer($key)) {
Copy link
Copy Markdown
Contributor

@Thevart Thevart Nov 1, 2017

Choose a reason for hiding this comment

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

@BertrandF23 we should add/update the tests for this change

Copy link
Copy Markdown
Contributor Author

@BertrandF23 BertrandF23 Nov 6, 2017

Choose a reason for hiding this comment

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

Tests are done for keyExists, testing true or false result, but also exceptions due to wrong type of parameters.

throw new \RuntimeException('keyExists expected either a string or an integer, \''.print_r($key, true).'\' was given.');
}

return array_key_exists($key, $obj);
}

Expand Down Expand Up @@ -302,11 +306,11 @@ public function serializeWithGroup($data, $format, $group)
}

/**
* Return the n-th section of the given string splitted by piece of the given length
* Return the n-th section of the given string splitted by piece of the given length.
*
* @param string $string
* @param int $length
* @param int $section
* @param int $length
* @param int $section
*
* @return string
*/
Expand All @@ -320,6 +324,7 @@ public function wordWrap($string, $length, $section)
if (isset($lines[$section])) {
$result = $lines[$section];
}

return $result;
}

Expand Down