Mwalker has submitted this change and it was merged.
Change subject: Massage watchdog calls and inline documentation
......................................................................
Massage watchdog calls and inline documentation
Change-Id: Ic75be00ac820cc889059f815fa3b0273853ae677
---
M
sites/all/modules/oneoffs/201305_paypal_recurring/sorry_may2013_paypal_recurring.php
M sites/all/modules/wmf_communication/Job.php
M sites/all/modules/wmf_communication/Mailer.php
M sites/all/modules/wmf_communication/MailingTemplate.php
M sites/all/modules/wmf_communication/Recipient.php
M sites/all/modules/wmf_communication/Templating.php
M sites/all/modules/wmf_communication/Translation.php
7 files changed, 249 insertions(+), 24 deletions(-)
Approvals:
Mwalker: Looks good to me, approved
jenkins-bot: Verified
diff --git
a/sites/all/modules/oneoffs/201305_paypal_recurring/sorry_may2013_paypal_recurring.php
b/sites/all/modules/oneoffs/201305_paypal_recurring/sorry_may2013_paypal_recurring.php
index 2e5cca3..58befa3 100644
---
a/sites/all/modules/oneoffs/201305_paypal_recurring/sorry_may2013_paypal_recurring.php
+++
b/sites/all/modules/oneoffs/201305_paypal_recurring/sorry_may2013_paypal_recurring.php
@@ -11,6 +11,7 @@
function sorry_may2013_paypal_recurring_build_job() {
$dbs = module_invoke( 'wmf_civicrm', 'get_dbs' );
+ // Find all contributions affected by this screwup
$dbs->push( 'civicrm' );
$result = db_query( "
SELECT
@@ -67,6 +68,7 @@
function sorry_may2013_paypal_recurring_mark_thanked() {
$job_ran_date = '2013-08-14 00:00:00';
+ // TODO function Job::getAllRecipientsForStatus or Recipient:getQueued...
to do exactly this
$query = db_select( 'wmf_communication_recipient', 'r' );
$query->join( 'wmf_communication_job', 'j', 'r.job_id = j.id' );
$query->addField( 'r', 'vars' );
diff --git a/sites/all/modules/wmf_communication/Job.php
b/sites/all/modules/wmf_communication/Job.php
index 0a2ed28..74aeebc 100644
--- a/sites/all/modules/wmf_communication/Job.php
+++ b/sites/all/modules/wmf_communication/Job.php
@@ -2,6 +2,23 @@
use \Exception;
+/**
+ * Entity representing a single mailing job batch run
+ *
+ * A job can be created in small, decoupled steps, and intermediate values
+ * examined in the database.
+ *
+ * For example, here is the lifecycle of a typical job:
+ *
+ * // Create an empty mailing job, which will render letters using the
+ * // specified template.
+ * $job = Job::create( 'RecurringDonationsSnafuMay2013Template' );
+ *
+ * foreach ( $recipients as $contact_id => $email ) {
+ * $job
+ * // Trigger the batch run. Execution
+ * $job->run();
+ */
class Job {
protected $id;
protected $template;
@@ -15,7 +32,11 @@
$job = new Job();
$job->id = $id;
- watchdog( 'wmf_communication', t( "Retrieving mailing job :id from the
database.", array( ':id' => $id ) ), WATCHDOG_INFO );
+ watchdog( 'wmf_communication',
+ "Retrieving mailing job :id from the database.",
+ array( ':id' => $id ),
+ WATCHDOG_INFO
+ );
$row = db_select( 'wmf_communication_job' )
->fields( 'wmf_communication_job' )
->condition( 'id', $id )
@@ -38,6 +59,13 @@
return $job;
}
+ /**
+ * Reserve an empty Job record and sequence number.
+ *
+ * @param string $templateClass mailing template classname
+ *
+ * TODO: other job-wide parameters and generic storage
+ */
static function create( $templateClass ) {
$jobId = db_insert( 'wmf_communication_job' )
->fields( array(
@@ -45,6 +73,14 @@
) )
->execute();
+ watchdog( 'wmf_communication',
+ "Created a new job id :id, of type :template_class.",
+ array(
+ ':id' => $jobId,
+ ':template_class' => $templateClass,
+ ),
+ WATCHDOG_INFO
+ );
return Job::getJob( $jobId );
}
@@ -52,7 +88,11 @@
* Find all queued recipients and send letters.
*/
function run() {
- watchdog( 'wmf_communication', t( "Running mailing job ID :id...",
array( ':id' => $this->id ) ), WATCHDOG_INFO );
+ watchdog( 'wmf_communication',
+ "Running mailing job ID :id...",
+ array( ':id' => $this->id ),
+ WATCHDOG_INFO
+ );
$mailer = Mailer::getDefault();
$successful = 0;
@@ -86,13 +126,21 @@
}
if ( ( $successful + $failed ) === 0 ) {
- watchdog( 'wmf_communication', t( "The mailing job (ID :id) was
empty, or already completed.", array( ':id' => $this->id ) ), WATCHDOG_WARNING
);
+ watchdog( 'wmf_communication',
+ "The mailing job (ID :id) was empty, or already completed.",
+ array( ':id' => $this->id ),
+ WATCHDOG_WARNING
+ );
} else {
watchdog( 'wmf_communication',
- t( "Completed mailing job (ID :id), :successful letters
successfully sent, and :failed failed.",
- array( ':id' => $this->id, ':successful' => $successful,
':failed' => $failed )
+ "Completed mailing job (ID :id), :successful letters
successfully sent, and :failed failed.",
+ array(
+ ':id' => $this->id,
+ ':successful' => $successful,
+ ':failed' => $failed,
),
- WATCHDOG_INFO );
+ WATCHDOG_INFO
+ );
}
}
diff --git a/sites/all/modules/wmf_communication/Mailer.php
b/sites/all/modules/wmf_communication/Mailer.php
index a73c8c4..7977f22 100644
--- a/sites/all/modules/wmf_communication/Mailer.php
+++ b/sites/all/modules/wmf_communication/Mailer.php
@@ -1,7 +1,12 @@
<?php namespace wmf_communication;
+/**
+ * Must be implemented by every mailing engine
+ */
interface IMailer {
/**
+ * Enqueue an email into the external mailing system
+ *
* @param array $email All keys are required:
* from_address
* from_name
@@ -11,13 +16,23 @@
* subject
* to_address
* to_name
+ *
+ * @return boolean True if the mailing system accepted your message for
delivery
*/
function send( $email );
}
+/**
+ * Weird factory to get the default concrete Mailer.
+ */
class Mailer {
static public $defaultSystem = 'phpmailer';
+ /**
+ * Get the default Mailer
+ *
+ * @return IMailer instantiated default Mailer
+ */
static public function getDefault() {
switch ( self::$defaultSystem ) {
case 'phpmailer':
@@ -30,15 +45,26 @@
}
}
+/**
+ * Use the PHPMailer engine
+ */
class MailerPHPMailer implements IMailer {
function __construct() {
$path = implode(DIRECTORY_SEPARATOR,
array(variable_get('wmf_common_phpmailer_location', ''),
'class.phpmailer.php'));
- watchdog( 'wmf_communication', t( "Loading PHPMailer class from
:path", array( ':path' => $path ) ), WATCHDOG_INFO );
+ watchdog( 'wmf_communication',
+ "Loading PHPMailer class from :path",
+ array( ':path' => $path ),
+ WATCHDOG_INFO
+ );
require_once( $path );
}
function send( $email, $headers = array() ) {
- watchdog( 'wmf_communication', t( "Sending an email to :to_address,
using PHPMailer", array( ':to_address' => $email['to_address'] ) ),
WATCHDOG_DEBUG );
+ watchdog( 'wmf_communication',
+ "Sending an email to :to_address, using PHPMailer",
+ array( ':to_address' => $email['to_address'] ),
+ WATCHDOG_DEBUG
+ );
$mailer = new \PHPMailer( true );
@@ -65,6 +91,9 @@
}
}
+/**
+ * Use the Drupal mailing system
+ */
class MailerDrupal implements IMailer {
function send( $email ) {
$from = "{$email['from_name']} <{$email['from_address']}>";
diff --git a/sites/all/modules/wmf_communication/MailingTemplate.php
b/sites/all/modules/wmf_communication/MailingTemplate.php
index 60d6fea..237f22f 100644
--- a/sites/all/modules/wmf_communication/MailingTemplate.php
+++ b/sites/all/modules/wmf_communication/MailingTemplate.php
@@ -1,8 +1,9 @@
<?php namespace wmf_communication;
/**
- * An overridable template provider, which returns the subject string and body
- * template for a given recipient's preferred language.
+ * Template provider, responsible for the subject and body templates
+ *
+ * Ordinarily, you can extend the AbstractMailingTemplate.
*/
interface IMailingTemplate {
/**
@@ -17,27 +18,70 @@
}
/**
- * Most mailings will extend this class and simply define a subject key and
template path.
+ * Base template provider for normal mailing jobs
+ *
+ * Most mailings will extend this class and simply define the two abstract
methods,
+ * which return a subject message key and the base template name.
+ *
* Anything which will be customized on a per-recipient basis should be
controlled by this
- * class or in your derivative.
+ * class (or in your subclass).
*/
abstract class AbstractMailingTemplate implements IMailingTemplate {
+
+ /**
+ * Get the root directory to search for templates
+ *
+ * @return string path
+ */
abstract function getTemplateDir();
+ /**
+ * Get the base template name
+ *
+ * This name is transformed into a template file path by the Templating
factory.
+ *
+ * @return string base name, such as 'thank_you'
+ */
abstract function getTemplateName();
+ /**
+ * Job is sent from this email address
+ *
+ * @return string email address
+ */
function getFromAddress() {
return variable_get( 'thank_you_from_address', null );
}
+ /**
+ * Job is sent as this name
+ *
+ * @return string full name for From string
+ */
function getFromName() {
return variable_get( 'thank_you_from_name', null );
}
+ /**
+ * Get the rendered subject line
+ *
+ * @param Recipient $recipient the addressee
+ *
+ * @return string subject
+ */
function getSubject( $recipient ) {
return trim( $this->getBodyTemplate( $recipient )->render( 'subject' )
);
}
+ /**
+ * Get a template appropriate for this recipient
+ *
+ * Merges contact information into the template variables.
+ *
+ * @param Recipient $recipient the addressee
+ *
+ * @return Templating prepared template
+ */
function getBodyTemplate( $recipient ) {
$templateParams = array(
'name' => $recipient->getName(),
diff --git a/sites/all/modules/wmf_communication/Recipient.php
b/sites/all/modules/wmf_communication/Recipient.php
index 58c8f1c..704093c 100644
--- a/sites/all/modules/wmf_communication/Recipient.php
+++ b/sites/all/modules/wmf_communication/Recipient.php
@@ -3,8 +3,11 @@
use \Exception;
/**
- * Record linking a job and CiviCRM contact, contains all information needed to
- * generate and send a letter, and current delivery status.
+ * Link metadata between a mailing job and a CiviCRM contact
+ *
+ * This object contains all the information needed to generate and send a
+ * letter, including template parameters. It can be queried for the current
+ * delivery status.
*/
class Recipient {
protected $contactId;
@@ -20,12 +23,18 @@
* Get recipients for this job, in the "queued" status, up to $batchSize.
*
* Call this repeatedly until empty set is returned.
+ *
+ * @param integer $jobId
+ * @param integer $batchSize maximum number to return
+ * @param integer $queuedState fetch recipients in the given state
+ *
+ * @return array of Recipient
*/
- static function getQueuedBatch( $jobId, $batchSize = 100 ) {
+ static function getQueuedBatch( $jobId, $batchSize = 100, $queuedState =
'queued' ) {
$result = db_select( 'wmf_communication_recipient' )
->fields( 'wmf_communication_recipient' )
->condition( 'job_id', $jobId )
- ->condition( 'status', 'queued' )
+ ->condition( 'status', $queuedState )
->orderBy( 'queued_id' )
->range( 0, $batchSize )
->execute();
@@ -37,7 +46,23 @@
return $recipients;
}
+ /**
+ * Add a CiviCRM contact to a mailing job
+ *
+ * @param integer $jobId
+ * @param integer $contactId CiviCRM contact id
+ * @param array $vars serializable template parameters
+ */
static function create( $jobId, $contactId, $vars ) {
+ watchdog( 'wmf_communication',
+ "Adding contact :contact_id to job :job_id, with parameters :vars",
+ array(
+ 'job_id' => $jobId,
+ 'contact_id' => $contactId,
+ 'vars' => json_encode( $vars, JSON_PRETTY_PRINT ),
+ ),
+ WATCHDOG_INFO
+ );
db_insert( 'wmf_communication_recipient' )
->fields( array(
'job_id' => $jobId,
@@ -49,7 +74,11 @@
}
/**
+ * Parse database record into a Recipient object
+ *
* @param array $dbRow associative form of the db record for a single
recipient
+ *
+ * @return Recipient
*/
static protected function loadFromRow( $dbRow ) {
$recipient = new Recipient();
@@ -121,6 +150,10 @@
/**
* Usually, you will want to use a specific accessor like setFailed, above.
+ *
+ * If your job has custom recipient workflow states, set them using this
method.
+ *
+ * @param string $status
*/
function setStatus( $status ) {
db_update( 'wmf_communication_recipient' )
diff --git a/sites/all/modules/wmf_communication/Templating.php
b/sites/all/modules/wmf_communication/Templating.php
index 17e4258..27f6fad 100644
--- a/sites/all/modules/wmf_communication/Templating.php
+++ b/sites/all/modules/wmf_communication/Templating.php
@@ -14,7 +14,22 @@
protected $format;
protected $template_params;
+ /**
+ * Prepare a template for substitution.
+ *
+ * This is not a persistent entity.
+ *
+ * TODO: factory instead of ctor
+ * TODO: Reconsider ideal level at which to "prepare".
+ *
+ * @param string $templates_dir file path to search for the template
+ * @param string $template_name base name for the desired template
+ * @param string $lang_code rendering locale
+ * @param array $template_params fed to the template
+ * @param string $format initial rendering mode (TODO: review usage)
+ */
function __construct( $templates_dir, $template_name, $lang_code,
$template_params, $format = null ) {
+ // TODO: autoloaded Twig::get
if ( !function_exists( 'wmf_common_get_twig' ) ) {
module_load_include( 'inc', 'wmf_common', 'twig' );
}
@@ -34,8 +49,7 @@
}
/**
- * Construct a path from the given parameters, load the appropriate
- * template, and render.
+ * Load the desired template or a fallback, and render as a string
*
* For the parameters {template_name: thank_you, language: it, format:
txt}, we will look in the path
* ${templates_dir}/txt/thank_you.it.txt
@@ -57,31 +71,59 @@
return $template->render( $this->template_params );
}
+ /**
+ * Get a short identifier specific to a single template file
+ *
+ * @return string munged natural key of (template, language, format)
+ */
protected function key() {
return "{$this->template_name} / {$this->language} . {$this->format}";
}
+ /**
+ * Find the best available template file for the desired locale and load
+ *
+ * @return \Twig_Template
+ */
protected function loadTemplate() {
$language = $this->language;
do {
+ // TODO: encapsulate path strategy in a function so it can be
overridden
$template = $this->loadTemplateFile(
"{$this->format}/{$this->template_name}.{$language}.{$this->format}" );
if ( $template ) {
return $template;
}
- watchdog( 'wmf_communication', t( "Template :key not found in
language ':language', attempting next fallback...", array( ':key' =>
$this->key(), ':language' => $language ) ), WATCHDOG_INFO );
+ watchdog( 'wmf_communication',
+ "Template :key not found in language ':language', attempting
next fallback...",
+ array(
+ ':key' => $this->key(),
+ ':language' => $language ),
+ WATCHDOG_INFO
+ );
$language = Translation::next_fallback( $language );
} while ( $language );
- watchdog( 'wmf_communication', t( "Using universal language fallback
for template :key...", array( ':key' => $this->key() ) ), WATCHDOG_INFO );
+ watchdog( 'wmf_communication',
+ "Using universal language fallback for template :key...",
+ array( ':key' => $this->key() ),
+ WATCHDOG_INFO
+ );
return $this->loadTemplateFile(
"{$this->format}/{$this->template_name}.{$this->format}" );
}
+ /**
+ * Load a Twig template from the given filesystem path
+ *
+ * @param string $path absolute path, or path relative to configured Twig
include dirs
+ *
+ * @return Twig_Template
+ */
protected function loadTemplateFile( $path ) {
watchdog( 'wmf_communication',
- t( "Searching for template file at :path",
- array( ':path' => $path )
- ), WATCHDOG_DEBUG
+ "Searching for template file at :path",
+ array( ':path' => $path ),
+ WATCHDOG_DEBUG
);
try {
return $this->twig->loadTemplate( $path );
diff --git a/sites/all/modules/wmf_communication/Translation.php
b/sites/all/modules/wmf_communication/Translation.php
index e3ed7b1..a279529 100644
--- a/sites/all/modules/wmf_communication/Translation.php
+++ b/sites/all/modules/wmf_communication/Translation.php
@@ -2,8 +2,17 @@
use \Exception;
+/**
+ * Helper for language tag manipulation and a rudimentary MediaWiki i18n facade
+ *
+ * TODO: deprecate
+ */
class Translation {
- //TODO: get from MediaWiki
+ /**
+ * Given a specific locale, get the next most general locale
+ *
+ * TODO: get from LanguageTag library and refactor interface
+ */
static function next_fallback( $language ) {
$parts = preg_split( '/-_/', $language );
if ( count( $parts ) > 1 ) {
@@ -15,6 +24,18 @@
return 'en';
}
+ /**
+ * Fetch a MediaWiki message translated in the DonationInterface group
+ *
+ * @param $key message key
+ * @param $language MediaWiki language code
+ *
+ * @return string message contents
+ *
+ * TODO: No wikitext expansion?
+ * TODO: accept standard language tag and convert to MW
+ * TODO: generalize beyond DonationInterface
+ */
static function get_translated_message( $key, $language ) {
require_once drupal_get_path( 'module', 'wmf_common' ) .
'/MessageFile.php';
@@ -35,9 +56,15 @@
} while ( $language );
}
+ /**
+ * Convert unix locale to a two-digit language code
+ *
+ * TODO: the cheeze
+ */
static function normalize_language_code( $code ) {
$locale = explode( '_', $code );
if ( count( $locale ) == 0 ) {
+ // TODO: return null
return 'en';
}
if ( count( $locale ) == 1 ) {
--
To view, visit https://gerrit.wikimedia.org/r/114911
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic75be00ac820cc889059f815fa3b0273853ae677
Gerrit-PatchSet: 5
Gerrit-Project: wikimedia/fundraising/crm
Gerrit-Branch: master
Gerrit-Owner: Adamw <awi...@wikimedia.org>
Gerrit-Reviewer: Mwalker <mwal...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
