getgrav / grav-plugin-email

Grav Email Plugin
http://getgrav.org
MIT License
37 stars 29 forks source link

Config addons #8

Closed hctom closed 8 years ago

hctom commented 8 years ago

This pull request does the following:

IMPORTANT NOTE: This rework produces PHP errors if old config files contain only string values for from and to parameters. Due to a different data type now, their values have to be removed manually from confing/plugins/email.yaml to avoid these problems. Perhaps someone has a better idea on how to solve this?

rhukster commented 8 years ago

Yah that's no good. Can't have an 'enhancement' just break existing installs.

There needs to be some allowance for backwards compatibility to ensure people's sites just don't suddenly break when updating a plugin.

rhukster commented 8 years ago

My suggestion is create all new top level fields, email_from, email_to, so they don't clash. Then in the code, if there is no email_from, then look for the old from string.

also blueprints should be set to validate: loose to ensure old values are not wiped out on save. It's not the prettiest, but at some point we can remove the deprecated checks.

hctom commented 8 years ago

The backward compatibility is (almost) completely given, because the parameters themselves support the old value types during processing. So all sites using the email plugin will work as expected after updating the plugin. Things only break when saving the plugin's config form with the admin plugin, because of the data type change.

So to summarize, these are the value types that are allowed for all supported e-mail related parameers with my changes:

Single e-mail address string
to: info@example.com
Array of e-mail address strings
to:
  - info@example.com
  - info+1@example.com
  - info+2@example.com
Single e-mail address definition
to:
  mail: info@example.com
  name: Human-readable name
Multiple e-mail address definitions/strings
to:
  -
    mail: info@example.com
    name: Human-readable name
  -
    mail: info+2@example.com
    name: Human-readable name 2
  -
    info+3@example.com
  -
    info+4@example.com

As you can see, this will be almost impossible to cover with the new variable names you suggested. So I was wondering, if there is any chance to update a sites config automatically during plugin update to the new structure, but I guess this is not supported by Grav right now, or is it?

Another idea would be to create a new top-level key which groups all e-mail definitions like this:

emails:
  from: info@example.com
  to:
    mail: info@example.com
    name: Human-readable name
  reply_to:
    - info+2@example.com

What do you think?

hctom commented 8 years ago

I'm sorry, I messed up my branches... please see #10 for new pull request