I'm not sure if it makes sense for me to be the one to complete the proposed resolution as I would be working backwards from the work I've already done.
In my opinion, as described on the original issue, blank lines entered into the field get saved to config as blank array elements. IE entering in a bunch of spaces and line breaks results in config like so:
notification:
emails:
- ''
- ''
- ''
This then causes the issue, because these come back as a keyed array of empty values when sending out the notifications. empty() doesn't see this as an empty array.
I believe the solution should take into account that 1. empty lines not be recorded and 2. if all lines are empty, then the config should instead be stored as an empty array / sequence
notification:
emails: { }
This resolves the issue going forward, but not for sites that have already exported the incorrect config. I'm not sure if it makes sense to spread a wider net to capture empty values or rather correct the incorrect config.
I can apply the fix to HEAD. It looks like it would be near identical. But I will hold off in case, there is a better solution that's needed here.
As for tests in #3382510, it doesn't look like there were any. The issue was around introducing ConfigTarget(), but how it's been implemented on the Update Settings form is unique to it and the logic defined in it's from and to methods.
As suspected, this was probably introduced in 3382510. This MR should resolve two issues:
- Ensure that an empty values now save to update.settings as {} as they did before
- Ensure that any sites who's config has been saved as notification: emails: - '' register as an empty value