Add constrain to check if plugin can be published. #13696
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…need to do some verification before publishing plugin
…the form validation fails so that it will not publish plugin with errors. If already in publish state do not unpublish as it may cause other things to fail
…o that it will not cause issue
Co-authored-by: Miroslav Fedeleš <miroslav.fedeles@gmail.com>
…skar/mautic into add_plugin_can_publish
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 5.x #13696 +/- ##
============================================
+ Coverage 61.42% 61.52% +0.09%
- Complexity 34050 34091 +41
============================================
Files 2240 2245 +5
Lines 101773 101909 +136
============================================
+ Hits 62519 62703 +184
+ Misses 39254 39206 -48
|
escopecz
added
ready-to-test
escopecz
requested changes
Apr 30, 2024
| * | ||
| * @return array <string,mixed> | ||
| * | ||
| * @phpstan-ignore-next-line |
There was a problem hiding this comment.
Can you please add a comment at the end of this line why this needs to be ignored?
Comment on lines
+81
to
+87
| 'mautic.plugin.validator.can_publish_validator' => [ | ||
| 'class' => Mautic\PluginBundle\Form\Constraint\CanPublishValidator::class, | ||
| 'arguments' => [ | ||
| 'event_dispatcher', | ||
| ], | ||
| 'tag' => 'validator.constraint_validator', | ||
| ], |
There was a problem hiding this comment.
This shouldn't be necessary for M5. Is it? The tag can be added in services.php if the service isn't autoconfigured.
Comment on lines
+9
to
+16
| private string $message; | ||
| private bool $canPublish; | ||
|
|
||
| public function __construct(private int $value, private string $integrationName) | ||
| { | ||
| $this->canPublish = true; | ||
| $this->message = ''; | ||
| } |
There was a problem hiding this comment.
Suggested change
| private string $message; | |
| private bool $canPublish; | |
| public function __construct(private int $value, private string $integrationName) | |
| { | |
| $this->canPublish = true; | |
| $this->message = ''; | |
| } | |
| private string $message = true; | |
| private bool $canPublish = ''; | |
| public function __construct(private int $value, private string $integrationName) | |
| { | |
| } |
| @@ -133,4 +133,13 @@ final class PluginEvents | |||
| * @var string | |||
| */ | |||
| public const ON_PLUGIN_INSTALL = 'mautic.plugin.on_plugin_install'; | |||
|
|
|||
| /** | |||
| * The mautic.plugin.is_published_state_changing event is dispatched when a user tries to changes plugin is published state. | |||
There was a problem hiding this comment.
Suggested change
| * The mautic.plugin.is_published_state_changing event is dispatched when a user tries to changes plugin is published state. | |
| * The mautic.plugin.is_published_state_changing event is dispatched when a user tries to change the published state of a plugin. |
| foreach ($container->getDefinitions() as $definition) { | ||
| $class = (string) $definition->getClass(); | ||
|
|
||
| /** @phpstan-ignore-next-line */ |
There was a problem hiding this comment.
Please add a reason why the next line is ignored if it cannot be fixed.
| @@ -0,0 +1,92 @@ | |||
| <?php | |||
|
|
|||
There was a problem hiding this comment.
Suggested change
| declare(strict_types=1); | |
Comment on lines
+18
to
+19
| private MockObject $dispatcher; | ||
| private MockObject $event; |
There was a problem hiding this comment.
These should have a & the real type like MockObject&EventDispatcherInterface as it needs the methods from both. PHP 8 doesn't allow the & operator just yet so you'll have to add the real type to the comment along as the MockObject type you have there.
| mautic.plugin.field.required_mapping_missing="At least one required field is not mapped." | ||
| mautic.lead_list.not_allowed_plugin_publish="You are not allowed to publish plugin due to insufficient configurations." |
There was a problem hiding this comment.
It doesn't sound right. Should there be the or this?
Suggested change
| mautic.lead_list.not_allowed_plugin_publish="You are not allowed to publish plugin due to insufficient configurations." | |
| mautic.lead_list.not_allowed_plugin_publish="You are not allowed to publish this plugin due to insufficient configurations." |
escopecz
added
the
pending-feedback
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
When authorizing the plugin, if my user doesn't have particular access, I should receive an error and fail to authorize the plugin. This i new constrain which we need to add in any plugin which will not allow user to publish the plugin if user don't have sufficient access.
Also clear the access and refresh token whenever credentials are changed.
Steps to test this PR: