Allow multilple delivery_address addresses #121
Conversation
jorns
force-pushed
the
multiple_delivery_adresses
branch
from
ef4d71c to
a981458
Compare
| @@ -66,6 +66,10 @@ | |||
| </xsd:restriction> | |||
| </xsd:simpleType> | |||
|
|
|||
| <xsd:simpleType name="delivery-address"> | |||
There was a problem hiding this comment.
Build fails due to this update. Won't fail after it's published on http://symfony.com/schema/dic/swiftmailer/swiftmailer-1.0.xsd.
There was a problem hiding this comment.
this is not true. Symfony uses the local schema. the issue is that you defined a custom type, but you never use it
jorns
force-pushed
the
multiple_delivery_adresses
branch
2 times, most recently
from
dc131e4 to
04179c0
Compare
|
Solves #99 |
| @@ -112,6 +112,14 @@ private function getMailersNode() | |||
| ->end() | |||
| ->scalarNode('sender_address')->end() | |||
| ->scalarNode('delivery_address')->end() | |||
There was a problem hiding this comment.
This is not needed anymore with the fixXmlConfig() call.
There was a problem hiding this comment.
and then you should check below if the passed value isn't an array (and cast it then)
jorns
force-pushed
the
multiple_delivery_adresses
branch
2 times, most recently
from
8cec8f1 to
63c3427
Compare
|
@stof can you look over it again? |
| ->arrayNode('delivery_addresses') | ||
| ->beforeNormalization() | ||
| ->always() | ||
| ->then(function ($v) { return array_values((array) $v); }) |
There was a problem hiding this comment.
You don't have to make that change, when the value already is an array:
// ...
->beforeNormalization()
->ifString()
->then(function ($v) { return (array) $v; })
->end()// ...
There was a problem hiding this comment.
array_values normalizes the keys so I can use
$mailer['delivery_addresses'][0]There was a problem hiding this comment.
@xabbuh, so what do you think? Is it still a problem for you?
There was a problem hiding this comment.
I do not see how you would get keys that are not normalised without this.
There was a problem hiding this comment.
This is possible with a config like:
delivery_addresses: { a: [email protected], b: [email protected] }There was a problem hiding this comment.
Shouldn't something like this be forbidden given that we have a scalar prototype here?
There was a problem hiding this comment.
@xabbuh there is a long-standing bug in the Config component (but fixing it is a BC break, and I forogt it when we worked on 3.0, missing the opportunity for this): lists are reindexed numerically for prototyped nodes only when 2 configs need to be merged together. But if only 1 file is configuring it, we don't call array_values to reset keys.
There was a problem hiding this comment.
@stof Too bad, then let's keep it as is.
jorns
force-pushed
the
multiple_delivery_adresses
branch
from
63c3427 to
9968c18
Compare
|
+1 |
|
This feature is very much required as so far I don't see any possible way to set up more than one recipient, which is essential for development and testing. |
|
Thank you @jorns. |
This PR was merged into the 2.3-dev branch. Discussion ---------- Allow multilple delivery_address addresses Commits ------- 9968c18 Allow multilple delivery_address addresses
No description provided.