Allow the Mail Spool to Support Multiple entity types

+++ b/includes/simplenews.source.incundefined
@@ -328,7 +330,19 @@ class SimplenewsSpool implements SimplenewsSpoolInterface {
-    return variable_get('simplenews_source', 'SimplenewsSourceNode');
+    // Convert entity type into upper camel case and generate a class name.
+    $entity_type_words = explode('_', $spool_data->entity_type);
+    $class = 'SimplenewsSource';
+    foreach ($entity_type_words as $word) {
+      $class .= drupal_ucfirst(trim($word));
+    }
+
+    if (class_exists($class)) {
+      return $class;
+    }
+    else {
+      return 'SimplenewsSourceEntity';

I'd just introduce a second variable with a key like simplenews_source_$type. First check that, with the Node one as default for nodes and then fall back to the global setting if there's no specific override.

+++ b/simplenews.installundefined
@@ -1088,3 +1092,27 @@ function simplenews_update_7204() {
+  db_change_field('simplenews_mail_spool', 'nid', 'entity_id', array(
+        'description' => 'The entity id of this newsletter issue.',
+        'type' => 'int',
+        'not null' => TRUE,
+        'default' => 0,
+      ));
+  db_add_field('simplenews_mail_spool', 'entity_type', array(
+        'description' => 'The entity type of this newsletter issue.',
+        'type' => 'varchar',
+        'length' => 255,
+        'not null' => TRUE,
+        'default' => '',
+      ));
+
+  // Set every entity type to 'node' at this point.
+  db_update('simplenews_mail_spool')->fields(array('entity_type' => 'node'))->execute();

Indendation of the array should be like this:

  db_change_field('simplenews_mail_spool', 'nid', 'entity_id', array(
    'description' => 'The entity id of this newsletter issue.',
    'type' => 'int',
    'not null' => TRUE,
    'default' => 0,
  ));

And the db_update() should have the fields() and execute() on a separate line.

+++ b/simplenews.moduleundefined
@@ -409,7 +409,7 @@ function simplenews_node_delete($node) {
-  $count = simplenews_delete_spool(array('nid' => $node->nid));
+  $count = simplenews_delete_spool(array('entity_id' => $node->nid, 'entity_type' => 'node'));

Wondering if we want to continue supporting just nid here and then default to entity_type node for that.

+++ b/simplenews.moduleundefined
@@ -2524,10 +2526,10 @@ function simplenews_issue_status($node, $value = NULL) {
-function simplenews_issue_sent_count($node, $value = NULL) {
+function simplenews_issue_sent_count($entity, $entity_type, $value = NULL) {

The argument order is usually the other way round. Adding it and the end would only make sense if it were optional and default to node (but then it would have to be after $value).

+++ b/includes/simplenews.source.incundefined
@@ -328,7 +330,15 @@ class SimplenewsSpool implements SimplenewsSpoolInterface {
   protected function getSourceImplementation($spool_data) {
-    return variable_get('simplenews_source', 'SimplenewsSourceNode');
+    $default = ($spool_data->entity_type == 'node') ? 'SimplenewsSourceNode' : NULL;
+
+    $class = variable_get('simplenews_source_' . $spool_data->entity_type, $default);
+
+    if (empty($class)) {
+      $class = variable_get('simplenews_source', 'SimplenewsSourceEntity');
+    }
+

Maybe add a comment or two here to explain what we are doing, looks good to me otherwise.

+++ b/simplenews.installundefined
@@ -1088,3 +1092,29 @@ function simplenews_update_7204() {
+ * Replace nid with entity_type and entity_id columns in
+ * {simplenews_mail_spool}
+ * Set the entity_type column on existing rows to node

Missing a . Need to check how this looks on update.php, docblocks on update functions should be kept short (single line) if possible.

+++ b/simplenews.moduleundefined
@@ -2518,8 +2518,10 @@ function simplenews_issue_status($node, $value = NULL) {
  *
- * @param $node
+ * @param $entity
  *   The issue to get or set the status.
+ * @param $entity_type
+ *   Then entity_type of $entity.
  * @param $value
  *   (Optional) The sent count to be set or NULL to retrieve the current value.
  * @return
@@ -2527,10 +2529,10 @@ function simplenews_issue_status($node, $value = NULL) {

@@ -2527,10 +2529,10 @@ function simplenews_issue_status($node, $value = NULL) {
  *
  * @ingroup issue
  */
-function simplenews_issue_sent_count($node, $value = NULL) {
+function simplenews_issue_sent_count($entity_type, $entity, $value = NULL) {

Order of entity_type and entity in @params and function is not consistent.

Sorry for the long wait, this looks good. Commited.