-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[Workflow] [WIP] Add transition blockers. #24745
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.
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
Changes from all commits
9eb7052
3a2ab8e
83faa63
27be4a2
35e3a7f
dfedfc5
71c4667
edda4a9
90a50f7
670f06b
1e8a19e
3f1cfd8
8cdc711
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\Workflow\Exception; | ||
|
||
use Symfony\Component\Workflow\TransitionBlockerList; | ||
|
||
/** | ||
* Thrown by the workflow when a transition is not enabled. | ||
*/ | ||
class BlockedTransitionException extends LogicException | ||
{ | ||
private $transitionBlockerList; | ||
|
||
public function __construct(string $message, TransitionBlockerList $transitionBlockerList) | ||
{ | ||
parent::__construct($message); | ||
|
||
$this->transitionBlockerList = $transitionBlockerList; | ||
} | ||
|
||
public function getTransitionBlockerList(): TransitionBlockerList | ||
{ | ||
return $this->transitionBlockerList; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\Workflow\Exception; | ||
|
||
/** | ||
* Thrown by Workflow when an undefined transition is applied on a subject. | ||
*/ | ||
class UndefinedTransitionException extends LogicException | ||
{ | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,112 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\Workflow; | ||
|
||
/** | ||
* A reason why a transition cannot be performed for a subject. | ||
*/ | ||
class TransitionBlocker | ||
{ | ||
const REASON_TRANSITION_NOT_DEFINED = '80f2a8e9-ee53-408a-9dd8-cce09e031db8'; | ||
const REASON_TRANSITION_NOT_APPLICABLE = '19beefc8-6b1e-4716-9d07-a39bd6d16e34'; | ||
const REASON_TRANSITION_UNKNOWN = 'e8b5bbb9-5913-4b98-bfa6-65dbd228a82a'; | ||
|
||
private $message; | ||
private $code; | ||
|
||
/** | ||
* @var array This is useful if you would like to pass around the condition values, that | ||
* blocked the transition. E.g. for a condition "distance must be larger than | ||
* 5 miles", you might want to pass around the value of 5. | ||
*/ | ||
private $parameters; | ||
|
||
public function __construct(string $message, string $code, array $parameters = array()) | ||
{ | ||
$this->message = $message; | ||
$this->code = $code; | ||
$this->parameters = $parameters; | ||
} | ||
|
||
/** | ||
* Create a blocker, that says the transition cannot be made because it is undefined | ||
* in a workflow. | ||
* | ||
* @param string $transitionName | ||
* @param string $workflowName | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this PHP Doc is useless. Please remove it (same for other occurrence) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This specific one -- yes, it's useless. I don't find the remaining two useless, though.
I kept the docblock for If after this explanation you still would like me to remove these docblocks, then I'll remove them. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I was talking only about the parameter. |
||
* | ||
* @return static | ||
*/ | ||
public static function createNotDefined(string $transitionName, string $workflowName): self | ||
{ | ||
$message = sprintf('Transition "%s" is not defined in workflow "%s".', $transitionName, $workflowName); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since PHP 7.0, you can use: It's easier to read IMHO There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sprintf is used throughout symfony, as well as double quotes for quoting values in messages There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I used |
||
$parameters = array( | ||
'transitionName' => $transitionName, | ||
'workflowName' => $workflowName, | ||
); | ||
|
||
return new static($message, self::REASON_TRANSITION_NOT_DEFINED, $parameters); | ||
} | ||
|
||
/** | ||
* Create a blocker, that says the transition cannot be made because the subject | ||
* is in wrong place (i.e. status). | ||
* | ||
* @param string $transitionName | ||
* | ||
* @return static | ||
*/ | ||
public static function createNotApplicable(string $transitionName): self | ||
{ | ||
$message = sprintf('Transition "%s" cannot be made, because the subject is not in the required place.', $transitionName); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. transition name could be additionally passed as parameter. |
||
$parameters = array( | ||
'transitionName' => $transitionName, | ||
); | ||
|
||
return new static($message, self::REASON_TRANSITION_NOT_APPLICABLE, $parameters); | ||
} | ||
|
||
/** | ||
* Create a blocker, that says the transition cannot be made because of unknown | ||
* reason. | ||
* | ||
* This blocker code is chiefly for preserving backwards compatibility. | ||
* | ||
* @param string $transitionName | ||
* | ||
* @return static | ||
*/ | ||
public static function createUnknownReason(string $transitionName): self | ||
{ | ||
$message = sprintf('Transition "%s" cannot be made, because of unknown reason.', $transitionName); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. transition name could be additionally passed as parameter. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right. I guess I didn't eat my own dog food here. I've done what you mentioned in all three places. |
||
$parameters = array( | ||
'transitionName' => $transitionName, | ||
); | ||
|
||
return new static($message, self::REASON_TRANSITION_UNKNOWN, $parameters); | ||
} | ||
|
||
public function getMessage(): string | ||
{ | ||
return $this->message; | ||
} | ||
|
||
public function getCode(): string | ||
{ | ||
return $this->code; | ||
} | ||
|
||
public function getParameters(): array | ||
{ | ||
return $this->parameters; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the order of method make the diff harder to read and to review. Could you keep the same method order ?
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The order of methods didn't change. Github's diff tool just struggles to find the similarity, due to the amount of changes in this file. Screenshot of PhpStorm's diff tool:
Let me know, if I missed something.