Removes option to group by lists during export

assets/js/src/subscribers/importExport/export.js

@@ -17,7 +17,6 @@ define(
    jQuery(document).ready(function () { // eslint-disable-line func-names
      var segmentsContainerElement;
      var subscriberFieldsContainerElement;
-     var groupBySegmentOptionElement;
      var nextStepButton;
      var renderSegmentsAndFields;
      var subscribersExportTemplate;
@@ -43,7 +42,6 @@ define(
      // define reusable variables
      segmentsContainerElement = jQuery('#export_lists');
      subscriberFieldsContainerElement = jQuery('#export_columns');
-     groupBySegmentOptionElement = jQuery(':checkbox[name="option_group_by_list"]');
      nextStepButton = jQuery('a.mailpoet_export_process');
      renderSegmentsAndFields = function (container, data) { // eslint-disable-line func-names
        if (container.data('select2')) {
@@ -100,13 +98,6 @@ define(
          else {
            toggleNextStepButton('off');
          }
-
-         if (segmentsContainerElement.select2('data').length > 1 && window.exportData.groupBySegmentOption) {
-           jQuery('.mailpoet_group_by_list').show();
-         }
-         else if (window.exportData.groupBySegmentOption) {
-           jQuery('.mailpoet_group_by_list').hide();
-         }
        });
      };
 
@@ -133,7 +124,6 @@ define(
          action: 'processExport',
          data: JSON.stringify({
            export_format_option: exportFormat,
-           group_by_segment_option: (groupBySegmentOptionElement.is(':visible')) ? groupBySegmentOptionElement.prop('checked') : false,
            segments: (window.exportData.segments) ? segmentsContainerElement.val() : false,
            subscriber_fields: subscriberFieldsContainerElement.val()
          })

lib/Subscribers/ImportExport/Export/Export.php

@@ -14,7 +14,6 @@ use MailPoet\Util\XLSXWriter;
 
 class Export {
   public $export_format_option;
-  public $group_by_segment_option;
   public $segments;
   public $subscribers_without_segment;
   public $subscriber_fields;
@@ -30,7 +29,6 @@ class Export {
       set_time_limit(0);
     }
     $this->export_format_option = $data['export_format_option'];
-    $this->group_by_segment_option = $data['group_by_segment_option'];
     $this->segments = $data['segments'];
     $this->subscribers_without_segment = array_search(0, $this->segments);
     $this->subscriber_fields = $data['subscriber_fields'];
@@ -76,12 +74,10 @@ class Export {
     $format_CSV = function($row) {
       return '"' . str_replace('"', '\"', $row) . '"';
     };
+    $formatted_subscriber_fields[] = __('List', 'mailpoet');
     // add UTF-8 BOM (3 bytes, hex EF BB BF) at the start of the file for
     // Excel to automatically recognize the encoding
     fwrite($CSV_file, chr(0xEF) . chr(0xBB) . chr(0xBF));
-    if($this->group_by_segment_option) {
-      $formatted_subscriber_fields[] = __('List', 'mailpoet');
-    }
     fwrite(
       $CSV_file,
       implode(
@@ -97,9 +93,7 @@ class Export {
       $processed_subscribers += count($subscribers);
       foreach($subscribers as $subscriber) {
         $row = $this->formatSubscriberData($subscriber);
-        if($this->group_by_segment_option) {
+        $row[] = ucwords($subscriber['segment_name']);
-          $row[] = ucwords($subscriber['segment_name']);
-        }
         fwrite($CSV_file, implode(',', array_map($format_CSV, $row)) . "\n");
       }
       $offset += $this->subscriber_batch_size;
@@ -122,19 +116,14 @@ class Export {
         $current_segment = ucwords($subscriber['segment_name']);
         // Sheet header (1st row) will be written only if:
         // * This is the first time we're processing a segment
-        // * "Group by subscriber option" is turned AND the previous subscriber's
+        // * The previous subscriber's segment is different from the current subscriber's segment
-        // segment is different from the current subscriber's segment
         // Header will NOT be written if:
         // * We have already processed the segment. Because SQL results are not
         // sorted by segment name (due to slow queries when using ORDER BY and LIMIT),
         // we need to keep track of processed segments so that we do not create header
         // multiple times when switching from one segment to another and back.
-        if((!count($processed_segments) ||
+        if((!count($processed_segments) || $last_segment !== $current_segment) &&
-            ($last_segment !== $current_segment && $this->group_by_segment_option)
+          (!in_array($last_segment, $processed_segments) || !in_array($current_segment, $processed_segments))
-          ) &&
-          (!in_array($last_segment, $processed_segments) ||
-            !in_array($current_segment, $processed_segments)
-          )
         ) {
           $this->writeXLSX(
             $XLSX_writer,
@@ -165,12 +154,7 @@ class Export {
   }
 
   function writeXLSX($XLSX_writer, $segment, $data) {
-    return $XLSX_writer->writeSheetRow(
+    return $XLSX_writer->writeSheetRow(ucwords($segment), $data);
-      ($this->group_by_segment_option) ?
-        ucwords($segment) :
-        __('All Lists', 'mailpoet'),
-      $data
-    );
   }
 
   function getSubscribers($offset, $limit) {
@@ -207,7 +191,8 @@ class Export {
         )
       )
       ->filter('filterWithCustomFieldsForExport')
-      ->groupBy(Subscriber::$_table . '.id');
+      ->groupBy(Subscriber::$_table . '.id')
+      ->groupBy(Segment::$_table . '.id');
 
     if($this->subscribers_without_segment !== false) {
       // if there are subscribers who do not belong to any segment, use
@@ -230,9 +215,6 @@ class Export {
         ->selectExpr('MAX(' . Segment::$_table . '.name) as segment_name')
         ->whereIn(SubscriberSegment::$_table . '.segment_id', $this->segments);
     }
-    if($this->group_by_segment_option) {
-      $subscribers = $subscribers->groupBy(Segment::$_table . '.id');
-    }
     $subscribers = $subscribers
       ->whereNull(Subscriber::$_table . '.deleted_at')
       ->offset($offset)

tests/unit/Subscribers/ImportExport/Export/ExportTest.php

@@ -103,8 +103,6 @@ class ExportTest extends \MailPoetTest {
   function testItCanConstruct() {
     expect($this->export->export_format_option)
       ->equals('csv');
-    expect($this->export->group_by_segment_option)
-      ->equals(false);
     expect($this->export->segments)
       ->equals(
         array(
@@ -177,41 +175,32 @@ class ExportTest extends \MailPoetTest {
   function testItCanGetSubscribers() {
     $this->export->segments = array(1);
     $subscribers = $this->export->getSubscribers(0, 10);
-    expect(count($subscribers))->equals(2);
+    expect($subscribers)->count(2);
     $this->export->segments = array(2);
     $subscribers = $this->export->getSubscribers(0, 10);
-    expect(count($subscribers))->equals(2);
+    expect($subscribers)->count(2);
     $this->export->segments = array(
       1,
       2
     );
     $subscribers = $this->export->getSubscribers(0, 10);
-    expect(count($subscribers))->equals(3);
+    expect($subscribers)->count(4);
   }
 
-  function testItCanGroupSubscribersBySegments() {
+  function testItAlwaysGroupsSubscribersBySegments() {
-    $this->export->group_by_segment_option = true;
     $this->export->subscribers_without_segment = true;
     $subscribers = $this->export->getSubscribers(0, 10);
-    expect(count($subscribers))->equals(5);
+    expect($subscribers)->count(5);
   }
 
   function testItCanGetSubscribersOnlyWithoutSegments() {
     $this->export->segments = array(0);
     $this->export->subscribers_without_segment = true;
     $subscribers = $this->export->getSubscribers(0, 10);
-    expect(count($subscribers))->equals(1);
+    expect($subscribers)->count(1);
     expect($subscribers[0]['segment_name'])->equals('Not In Segment');
   }
 
-  function testItCanGetSubscribersOnlyInSegments() {
-    SubscriberSegment::where('subscriber_id', 3)
-      ->findOne()
-      ->delete();
-    $subscribers = $this->export->getSubscribers(0, 10);
-    expect(count($subscribers))->equals(2);
-  }
-
   function testItRequiresWritableExportFile() {
     try {
       $this->export->export_path = '/fake_folder';
@@ -231,7 +220,7 @@ class ExportTest extends \MailPoetTest {
     } catch(\Exception $e) {
       $this->fail('Export to .csv process threw an exception');
     }
-    expect($result['totalExported'])->equals(3);
+    expect($result['totalExported'])->equals(4);
     expect($result['exportFileURL'])->notEmpty();
 
     try {
@@ -241,7 +230,7 @@ class ExportTest extends \MailPoetTest {
     } catch(\Exception $e) {
       $this->fail('Export to .xlsx process threw an exception');
     }
-    expect($result['totalExported'])->equals(3);
+    expect($result['totalExported'])->equals(4);
     expect($result['exportFileURL'])->notEmpty();
   }
 

tests/unit/Subscribers/ImportExport/Export/ExportTestData.json

@@ -1,6 +1,5 @@
 {
   "export_format_option": "csv",
-  "group_by_segment_option": false,
   "segments": [
     "1",
     "2"

views/subscribers/importExport/export.html

@@ -89,8 +89,7 @@
     subscriberFieldsSelect2 =
       <%= subscriberFieldsSelect2|raw %>,
     exportData = {
-     segments: segments.length || null,
+     segments: segments.length || null
-     groupBySegmentOption: segments.length > 1 === true
     };
 </script>
 <% endblock %>