diff --git a/assets/js/src/subscribers/importExport/export.js b/assets/js/src/subscribers/importExport/export.js index b6af770de8..3725bff925 100644 --- a/assets/js/src/subscribers/importExport/export.js +++ b/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() }) diff --git a/lib/Subscribers/ImportExport/Export/Export.php b/lib/Subscribers/ImportExport/Export/Export.php index 19c7823059..ec256d1e86 100644 --- a/lib/Subscribers/ImportExport/Export/Export.php +++ b/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 - // segment is different from the current subscriber's segment + // * The previous subscriber's 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) || - ($last_segment !== $current_segment && $this->group_by_segment_option) - ) && - (!in_array($last_segment, $processed_segments) || - !in_array($current_segment, $processed_segments) - ) + if((!count($processed_segments) || $last_segment !== $current_segment) && + (!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( - ($this->group_by_segment_option) ? - ucwords($segment) : - __('All Lists', 'mailpoet'), - $data - ); + return $XLSX_writer->writeSheetRow(ucwords($segment), $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) diff --git a/tests/unit/Subscribers/ImportExport/Export/ExportTest.php b/tests/unit/Subscribers/ImportExport/Export/ExportTest.php index 20bedd1609..b6eeb806bf 100644 --- a/tests/unit/Subscribers/ImportExport/Export/ExportTest.php +++ b/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() { - $this->export->group_by_segment_option = true; + function testItAlwaysGroupsSubscribersBySegments() { $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(); } diff --git a/tests/unit/Subscribers/ImportExport/Export/ExportTestData.json b/tests/unit/Subscribers/ImportExport/Export/ExportTestData.json index 919aafd1ce..51d47caecb 100644 --- a/tests/unit/Subscribers/ImportExport/Export/ExportTestData.json +++ b/tests/unit/Subscribers/ImportExport/Export/ExportTestData.json @@ -1,6 +1,5 @@ { "export_format_option": "csv", - "group_by_segment_option": false, "segments": [ "1", "2" diff --git a/views/subscribers/importExport/export.html b/views/subscribers/importExport/export.html index e29b0a40aa..0b181c8d5f 100644 --- a/views/subscribers/importExport/export.html +++ b/views/subscribers/importExport/export.html @@ -89,8 +89,7 @@ subscriberFieldsSelect2 = <%= subscriberFieldsSelect2|raw %>, exportData = { - segments: segments.length || null, - groupBySegmentOption: segments.length > 1 === true + segments: segments.length || null }; <% endblock %>