When using different classes for formatting and delivering mail messages, mailsystem generates classes dynamically and stores them into the files directory. This patch provides a method which does not relly on generated classes. Instead the right implementation is selected from within a generic delegator class based on configuration stored as drupal variables.

This patch also would resolve #1369736: Alternative locations for "created" Class files? and probably #1359482: Error enabling modules that use mailsystem in second and thereafter attempts..

I'm however not quite sure how the administrative interface should work with this approach. Ideas?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

znerol’s picture

Ok, another round of patches. This set includes a rewrite of the administrative interface.

pillarsdotnet’s picture

Status: Active » Needs review

Code looks good at first glance and part of it is exactly what I was thinking about writing, myself.

Assuming all this applies and tests out, would you like to be a co-maintainer?

pillarsdotnet’s picture

Status: Needs review » Needs work
FileSize
23.12 KB

Code needs a bit of work, at second glance. At a minimum, the newly-added functions need to be documented according to coding standards.

Here's a re-roll relative to the 7.x-2.34 release.

Will probably start a 7.x-3.x branch for this, as it's both a UI and an API change.

znerol’s picture

Added missing headerdoc strings, improved validation when creating new module-specific mailsystem-settings, Fixed a syntax error (superflous closing brace in function mailsystem_admin_settings.

znerol’s picture

Status: Needs work » Needs review
FileSize
27.6 KB

Another take: This patch now includes an update-function. When updating generated classes are removed but the configuration will be kept. Furthermore I've reintroduced the theme-chooser in the admin-page. I've forgotten that bit in the previous patches.

sbrattla’s picture

I've tested the patch, and it works well for me. I'm working on integrating my module Swift Mailer with mailsystem, but I'm a little reluctant to do it before at least this patch gets comitted. My entire testsite went down before I patched mailsystem with this patch.

pillarsdotnet’s picture

Thanks for testing.

sbrattla’s picture

No problem, looking forward to seeing it commited :-)

sbrattla’s picture

Just one more question with regards to this one; is there anything I can do to make this patch to go through?

pillarsdotnet’s picture

I'll be able to work on it Tuesday -- got a hard deadline to meet and won't be free before then.

sbrattla’s picture

Sounds great :-)

I'll be glad to help if there is anything I can do (test any further updates etc.)

mrfelton’s picture

Working for me, thanks.

pillarsdotnet’s picture

Assigned: Unassigned » pillarsdotnet

Reviewing now...

pillarsdotnet’s picture

Yup. This is going into a 7.x-3.x release.

pillarsdotnet’s picture

Status: Needs review » Fixed

Okay, pushed to 7.x-3.x. Will integrate additional code before committing a new release:

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.