-
Notifications
You must be signed in to change notification settings - Fork 12
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
feat: add Webhook component integration #120
base: main
Are you sure you want to change the base?
feat: add Webhook component integration #120
Conversation
9b8c0c4
to
c5809a8
Compare
c5809a8
to
8c8d531
Compare
@@ -23,6 +23,7 @@ | |||
service('sensiolabs_gotenberg.asset.base_dir_formatter'), | |||
service('.sensiolabs_gotenberg.webhook_configuration_registry'), | |||
service('request_stack'), | |||
service('router')->nullOnInvalid(), |
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.
Still no desire to clean this file ? 👼
$services->set('.sensiolabs_gotenberg.builder')
->abstract()
->args([
service('sensiolabs_gotenberg.client'),
service('sensiolabs_gotenberg.asset.base_dir_formatter'),
service('.sensiolabs_gotenberg.webhook_configuration_registry'),
service('request_stack'),
service('router')->nullOnInvalid(),
])
->call('setLogger', [service('logger')->nullOnInvalid()])
;
$services->set('.sensiolabs_gotenberg.pdf_builder.markdown', MarkdownPdfBuilder::class)
->parent('.sensiolabs_gotenberb.builder')
->share(false)
->tag('sensiolabs_gotenberg.pdf_builder')
;
// ...
Or... even better: introduce a BuilderFactory ?
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.
Still no desire to clean this file ? 👼
I want to clean this file. I did it in a PoC about refactoring builder: #118
Or... even better: introduce a BuilderFactory ?
I also planned to test that 😉
|
||
return new SuccessGotenbergEvent( | ||
$id, | ||
$request->getContent(true), |
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.
Is messenger able to serialize a resource ?
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.
Good catch!
I'll make some tests. It coud depend on the transport used (ex: 512Mb fot RabbitMQ)
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.
hmmmm. That would mean that we might need to save the file beforehand ?
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.
I don't get how it works... the "resource" here is some form POST data, right ? So there is a limit in Symfony itself to receive the form data, no ?
public function __construct( | ||
string $id, | ||
mixed $file, | ||
private readonly string $filename, | ||
private readonly string $contentType, | ||
private readonly int $contentLength, | ||
) { | ||
$payload['file'] = $file; | ||
|
||
parent::__construct(self::SUCCESS, $id, $payload); | ||
} |
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.
Not sure i would create array like this :|
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.
Wow. I missed that one 😄
Besides, is it still relevant to add the file in $payload
instead of adding an other private property?
Co-authored-by: Simon André <[email protected]>
Co-authored-by: Simon André <[email protected]>
Co-authored-by: Simon André <[email protected]>
throw new MissingRequiredFieldException('->webhookUrl() was never called.'); | ||
} | ||
$successWebhookUrl = $this->successWebhookUrl; | ||
if (!$successWebhookUrl) { |
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.
strict ?
} | ||
$successWebhookUrl = $this->successWebhookUrl; | ||
if (!$successWebhookUrl) { | ||
if (!$this->urlGenerator) { |
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.
strict ?
@@ -30,9 +31,10 @@ public function __construct( | |||
AssetBaseDirFormatter $asset, | |||
WebhookConfigurationRegistryInterface $webhookConfigurationRegistry, | |||
private readonly RequestStack $requestStack, | |||
UrlGeneratorInterface|null $urlGenerator = null, |
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.
better to be consistent with parent construct. I suggest to switch with the request stack.
@@ -27,9 +28,10 @@ public function __construct( | |||
AssetBaseDirFormatter $asset, | |||
WebhookConfigurationRegistryInterface $webhookConfigurationRegistry, | |||
private readonly RequestStack $requestStack, | |||
UrlGeneratorInterface|null $urlGenerator = null, |
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.
switch with request stack
|
||
return new SuccessGotenbergEvent( | ||
$id, | ||
$request->getContent(true), |
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.
hmmmm. That would mean that we might need to save the file beforehand ?
{ | ||
"status": 500, | ||
"message": "An error occurred." | ||
} |
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.
EOF
Add Webhook component integration.
Webhook URLs fallback to the component's one (
/webhook/gotenberg
) if no configuration is set.