The attached patch provides two new actions to simplenews_rules

  1. simplenews_rules_action_subscription_delete ( Delete a subscriber from a selected newsletter )
  2. simplenews_rules_action_subscriber_delete ( Delete a subscriber from all newsletters )

Thanks,

DT

CommentFileSizeAuthor
#7 1612508-simplenews-rules-delete-subscription-action-v2.patch1.64 KBdavidwhthomas
#4 1612508-simplenews-rules-delete-subscription-action.patch3.42 KBdavidwhthomas
simplenews-rules-delete-subscription-action.patch2.32 KBdavidwhthomas
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs review » Needs work

Looks ok to me, one small coding style issue below. Wondering if we want to reference the unsubscribe action somehow in the description, stating that you should only use delete if you really want that information to be gone completely, or something like that.

+++ b/simplenews_rules/simplenews_rules.rules.incundefined
@@ -109,6 +109,34 @@ function simplenews_rules_rules_action_info() {
+      'label' => t('Delete an e-mail address from a newsletter'),
+      'group' => t('Simplenews'),      'named parameter' => TRUE,

The named parameter part should be on a separate line.

Same for the one below.

davidwhthomas’s picture

davidwhthomas CreditAttribution: davidwhthomas commented

Thanks Berdir,

I noticed the named parameter on the same line for the other existing actions, so just kept that format, in case was for a reason.

I'll make some adjustments and post an update.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

Wasn't aware that the existing rules were wrong as well. You're welcome to fix that as well while you're at it.

davidwhthomas’s picture

davidwhthomas CreditAttribution: davidwhthomas commented
Version: 7.x-1.0-beta2 » 7.x-1.x-dev
Status: Needs work » Needs review
FileSize
1612508-simplenews-rules-delete-subscription-action.patch3.42 KB

Thanks Berdir, there's the updated patch.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs review » Fixed

Thanks, commited and pushed!

Status: Fixed » Closed (fixed)

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

davidwhthomas’s picture

davidwhthomas CreditAttribution: davidwhthomas commented
Status: Closed (fixed) » Needs review
FileSize
1612508-simplenews-rules-delete-subscription-action-v2.patch1.64 KB

Hi Berdir,

I found a bug with the earlier rules patch when deleting a subscription from a specific newsletter.

The rule was passing the subscriber 'mail' when it needed to pass the subscriber 'snid'

The attached patch fixes that issue.

Thanks.

dlnsk’s picture

dlnsk CreditAttribution: dlnsk commented

I made same changes for simplenews_rules_action_subscription_delete

areynolds’s picture

areynolds CreditAttribution: areynolds commented

Patch in #7 works for me on 7.x-1.1; since both dev and 1.1 are probably at the same point right now, guessing this is good to be committed.

areynolds’s picture

areynolds CreditAttribution: areynolds commented

Actually @davidwhthomas, is there any reason for the changes in simplenews_mail_spool() that are included in the patch? May want to re-roll with only the changes in simplenews_rules_action_subscription_delete(). Sorry to nitpick, but guessing the maintainer may appreciate that if it is extraneous.

  • Berdir committed 5497565 on 8.x-1.x authored by davidwhthomas 
    Issue #1612508 by davidwhthomas: Added Rules actions for subscriber and...