feat: Add downloadFrom new fields (release 8.28)#256
Conversation
StevenRenaux
force-pushed
the
feat/Release-8.28-Add-downloadFrom-new-fields
branch
from
3e97eeb to
9739b7f
Compare
| * Sets download from to download each entry (file) in parallel (URLs MUST return a Content-Disposition header with a filename parameter.). | ||
| * | ||
| * @param list<array{url: string, extraHttpHeaders?: array<string, string>}> $downloadFrom | ||
| * @param list<array{url: string, extraHttpHeaders?: array<string, string>, field?: ''|'watermark'|'stamp'|'embedded'}> $downloadFrom |
There was a problem hiding this comment.
Note for next major version, we could propose constants to improve DX. (constant usage in traits was introduced in PHP 8.2 but Symfony 6.4 supports >=8.1).
There was a problem hiding this comment.
If it is a fixed list maybe we could create an enum directly ? It would help giving sense to ''
There was a problem hiding this comment.
Not totally agree, if we do that we need a special normalizer. And if we do that, we need to also improve this logic on some others node as :
https://github.com/sensiolabs/GotenbergBundle/blob/1.x/src/Builder/Behaviors/MetadataTrait.php#L59
https://github.com/sensiolabs/GotenbergBundle/blob/1.x/src/Builder/Behaviors/WebhookTrait.php#L59
There was a problem hiding this comment.
We already have a EnumNormalizer to convert Enum to string.
About traits you listed, Metadata key can be anything (see #245) and Cookie/Webhook are not related to Gotenberg
There was a problem hiding this comment.
Used in this context when the value is an argument to the method, and also using NativeEnumNodeBuilder in the configuration node.
In our case, to do this, during JSON normalization, we need to check if it's an enumeration, and then call the corresponding method in the supplement.
There was a problem hiding this comment.
And a dedicated NormalizerFactory::download ?
Neirda24
left a comment
Neirda24
left a comment
There was a problem hiding this comment.
After fixes asked form Hubert 👍
| * Sets download from to download each entry (file) in parallel (URLs MUST return a Content-Disposition header with a filename parameter.). | ||
| * | ||
| * @param list<array{url: string, extraHttpHeaders?: array<string, string>}> $downloadFrom | ||
| * @param list<array{url: string, extraHttpHeaders?: array<string, string>, field?: ''|'watermark'|'stamp'|'embedded'}> $downloadFrom |
There was a problem hiding this comment.
If it is a fixed list maybe we could create an enum directly ? It would help giving sense to ''
StevenRenaux
force-pushed
the
feat/Release-8.28-Add-downloadFrom-new-fields
branch
from
c1c5de7 to
1a5ff97
Compare
StevenRenaux
force-pushed
the
feat/Release-8.28-Add-downloadFrom-new-fields
branch
from
1a5ff97 to
1ac2058
Compare
3 participants
Description
https://github.com/gotenberg/gotenberg/releases/tag/v8.28.0