Fix newsletter criteria not always selecting all articles

The limit applied in the subquery could cause, that inner join
joined top 5 most read articles within the period, and then
the parent query filtered articles published within the
selected period.

That effectivelly caused that if one of the most read articles
was published outside of the selected period, criterion
excluded it and didn't deliver required number of articles.

remp/respekt#378

Author: rootpd

Beam/extensions/beam-module/src/Model/Article.php

@@ -330,13 +330,16 @@ public function scopeMostReadByPageviews(Builder $query, Carbon $start, string $
             ->select(['article_id', DB::raw("sum($getBy) as total_sum")])
             ->orderByDesc('total_sum');
 
+        $query = $query->joinSub($innerQuery, 't', function ($join) {
+                $join->on('articles.id', '=', 't.article_id');
+            })
+            ->orderByDesc('t.total_sum');
+
         if ($limit) {
-            $innerQuery->limit($limit);
+            $query = $query->limit($limit);
         }
 
-        return $query->joinSub($innerQuery, 't', function ($join) {
-            $join->on('articles.id', '=', 't.article_id');
-        })->orderByDesc('t.total_sum');
+        return $query;
     }
 
     public function scopeMostReadByAveragePaymentAmount(Builder $query, Carbon $start, ?int $limit = null): Builder
@@ -346,13 +349,15 @@ public function scopeMostReadByAveragePaymentAmount(Builder $query, Carbon $star
             ->select(['article_id', DB::raw('avg(amount) as average')])
             ->orderByDesc('average');
 
+        $query = $query->joinSub($innerQuery, 'c', function ($join) {
+            $join->on('articles.id', '=', 'c.article_id');
+        })->orderByDesc('c.average');
+
         if ($limit) {
-            $innerQuery->limit($limit);
+            $query = $query->limit($limit);
         }
 
-        return $query->joinSub($innerQuery, 'c', function ($join) {
-            $join->on('articles.id', '=', 'c.article_id');
-        })->orderByDesc('c.average');
+        return $query;
     }
 
     public function scopeMostReadByTotalPaymentAmount(Builder $query, Carbon $start, ?int $limit = null): Builder
@@ -362,13 +367,15 @@ public function scopeMostReadByTotalPaymentAmount(Builder $query, Carbon $start,
             ->select(['article_id', DB::raw('sum(amount) as average')])
             ->orderByDesc('average');
 
+        $query = $query->joinSub($innerQuery, 'c', function ($join) {
+            $join->on('articles.id', '=', 'c.article_id');
+        })->orderByDesc('c.average');
+
         if ($limit) {
-            $innerQuery->limit($limit);
+            $query = $query->limit($limit);
         }
 
-        return $query->joinSub($innerQuery, 'c', function ($join) {
-            $join->on('articles.id', '=', 'c.article_id');
-        })->orderByDesc('c.average');
+        return $query;
     }
 
     public function scopeIgnoreAuthorIds(Builder $query, array $authorIds): Builder

CHANGELOG.md

@@ -11,7 +11,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
 - **IMPORTANT** Removed data of sections "Visitors" and "Google Analytics Reporting", which we removed in the previous version. remp/remp#1349
   - Migration removes tables `session_devices` and `session_referers`.
 - Changed banner preview components as now include and run custom JS inside of iframe. remp/crm#3353
-- Added option to specify api version for gender balance in env. remp/helpdesk#3303 
+- Added option to specify api version for gender balance in env. remp/helpdesk#3303
+- Fixed newsletter criteria not always selecting the desired number of articles. remp/respekt#378
 
 ### [Campaign]