CiviCRM Community Forums (archive)

Looks pretty cool! I haven't had a chance to test it, but a few low-level suggestions/comments from skimming the code:

 * In https://github.com/JohnFF/Email-Amender/blob/master/emailamender.php#L26 , the function create_activity_type_if_doesnt_exist should be prefixed (eg "emailamender_create_activity_type_if_doesnt_exist()" or "_emailamender_create_activity_type_if_doesnt_exist()").

 * In https://github.com/JohnFF/Email-Amender/blob/master/templates/CRM/Emailamender/Page/EmailAmenderSettings.js , it adds several functions to the global namespace. This probably works now but can produce conflicts when scaled to lots of extensions. It's generally better practice to put JS inside a closure or the CRM object, e.g.

// Example 1: Putting myFunc in a closure
cj(function($) {
  var myFunc = function(...) { ...}

  $('.my-selector').whatever(...);
});

// Example 2: Putting myFunc in CRM
var CRM = CRM || {};
CRM.EmailAmender = CRM.EmailAmender || {};
CRM.EmailAmender.myFunc = function(...) { ... }

 * The install/uninstall does some work to manage "settings". For future reference, hook_civicrm_alterSettingsFolders ( http://wiki.civicrm.org/confluence/display/CRMDOC/hook_civicrm_alterSettingsFolders ) provides a way to declaratively define settings (using files like https://github.com/civicrm/civicrm-core/blob/master/settings/Core.setting.php ). This has the upshot of integrating better with the Settings API and CiviConfigure. Of course, if you've already tested the current setup process, then it's probably better to stick with your current code. But next time you need to declare some settings, give it a try.