- Issue created by @Grevil
- Status changed to Needs work
about 1 year ago 1:14pm 16 October 2023 - @lrwebks opened merge request.
- Assigned to lrwebks
- Status changed to Needs review
about 1 year ago 7:59am 20 October 2023 - 🇩🇪Germany lrwebks Porta Westfalica
Here we go, works for me! Please review!
- 🇩🇪Germany Anybody Porta Westfalica
@LRWebks: Nice! Here comes the trap: Changing form structure may lead to broken validation / submit logic (that's based on the #tree property on form arrays). If it is a tree, you may need to also adjust the handling logic. Same may happen for configs!
So please try, if it still works as expected. Don't rely on test coverage for modules and please provide information if and how much you tested, when setting an issue to "Needs review", please. So we're informed how safe it is and if it needs further (manual or automated) testing. Welcome to the next level :P
- Status changed to Needs work
about 1 year ago 8:30am 20 October 2023 - Status changed to Needs review
about 1 year ago 10:11am 20 October 2023 - 🇩🇪Germany lrwebks Porta Westfalica
As far as I have searched through the code, all classes that use these settings get them directly from the configFactory via
$container->get('config.factory')->get('email_registration.settings')
, the previous fieldset that they were in was never mentioned anywhere, so I guess the fieldset doesn't matter if it is done this way?
I see no need to change anything - manual tests work, automated tests work... - Assigned to Grevil
- Issue was unassigned.
- Status changed to RTBC
about 1 year ago 2:36pm 23 October 2023 - 🇩🇪Germany Grevil
LGTM! Sry, I said "fieldset", but I meant "details" wrapper. Small adjustments, everything else is perfect!
- Status changed to Fixed
about 1 year ago 2:38pm 23 October 2023 Automatically closed - issue fixed for 2 weeks with no activity.