Blocked users count towards mailing list subscriptions.

CommentFileSizeAuthor
#9 blocked_with_tests.patch2.74 KBberdir
#7 1243232-blocked-users.patch940 bytesngstigator
#4 1243232-blocked-users.patch926 bytesngstigator
#1 1243232-blocked-users.patch103 bytesngstigator

Comments

ngstigator’s picture

ngstigator commented
StatusFileSize
new 1243232-blocked-users.patch103 bytes

patch attached

simon georges’s picture

simon georges commented
Status: Active » Needs review
simon georges’s picture

simon georges commented
Status: Needs review » Needs work

The patch needs work (it doesn't even include what file to patch ;-)).

ngstigator’s picture

ngstigator commented
StatusFileSize
new 1243232-blocked-users.patch926 bytes

blocking a user already deactivates the subscriber, so rerolled to join just the 'simplenews_subscriber' table.

and sorry i didn't diff against the working copy (duh!) :-)

chris

simon georges’s picture

simon georges commented
Status: Needs work » Needs review

Ok, thanks, needs review, then ;-)

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work 
+++ b/includes/simplenews.admin.inc
@@ -1719,11 +1719,13 @@ function simplenews_count_subscriptions($tid) {
-  $subscription_count[$tid] = db_select('simplenews_subscription')
-    ->condition('tid', $tid)
-    ->condition('status', SIMPLENEWS_SUBSCRIPTION_STATUS_SUBSCRIBED)
-    ->countQuery()->execute()->fetchField();
-  return $subscription_count[$tid];
+  $query = db_select('simplenews_subscription', 'ss');
+  $query->leftJoin('simplenews_subscriber', 'sn', 'sn.snid = ss.snid');
+  $query->condition('tid', $tid)
+    ->condition('sn.activated', 1)
+    ->condition('status', SIMPLENEWS_SUBSCRIPTION_STATUS_SUBSCRIBED);
+
+  return $query->countQuery()->execute()->fetchField();

The new code doesn't set/use the static cache ($subscriptions_count).

ngstigator’s picture

ngstigator commented
Status: Needs work » Needs review
StatusFileSize
new 1243232-blocked-users.patch940 bytes

rerolled against latest 7.x-1.x-dev and modified to set $subscription_count[$tid].

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Tagging for test candicates

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
StatusFileSize
new blocked_with_tests.patch2.74 KB

Added tests for the patch.

berdir’s picture

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

Commited.

Status: Fixed » Closed (fixed)

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

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented

This issue seems also to be well coverd by tests. Untagging.