Merge pull request #10199 from PiwikPRO/PCC-7

Adds confirmation when changing segments definition

7 files changed, 88 insertions, 4 deletions

diff --git a/core/ArchiveProcessor/Rules.php b/core/ArchiveProcessor/Rules.php
index 7f37b3250c..7538d7f7d0 100644
--- a/core/ArchiveProcessor/Rules.php
+++ b/core/ArchiveProcessor/Rules.php
@@ -163,6 +163,12 @@ class Rules
         return Config::getInstance()->General['time_before_today_archive_considered_outdated'];
     }
 
+    public static function isBrowserArchivingAvailableForSegments()
+    {
+        $generalConfig = Config::getInstance()->General;
+        return (self::isRequestAuthorizedToArchive() && !$generalConfig['browser_archiving_disabled_enforce']);
+    }
+
     public static function isArchivingDisabledFor(array $idSites, Segment $segment, $periodLabel)
     {
         $generalConfig = Config::getInstance()->General;
diff --git a/plugins/SegmentEditor/SegmentSelectorControl.php b/plugins/SegmentEditor/SegmentSelectorControl.php
index 130b616d48..560af449fa 100644
--- a/plugins/SegmentEditor/SegmentSelectorControl.php
+++ b/plugins/SegmentEditor/SegmentSelectorControl.php
@@ -9,11 +9,13 @@
 namespace Piwik\Plugins\SegmentEditor;
 
 use Piwik\API\Request;
+use Piwik\ArchiveProcessor\Rules;
 use Piwik\Common;
 use Piwik\Config;
 use Piwik\Container\StaticContainer;
 use Piwik\Piwik;
 use Piwik\Plugins\API\API as APIMetadata;
+use Piwik\Plugins\UsersManager\API AS UsersManagerAPI;
 use Piwik\View\UIControl;
 use Piwik\Plugins\SegmentEditor\API as SegmentEditorAPI;
 
@@ -78,6 +80,8 @@ class SegmentSelectorControl extends UIControl
         $this->authorizedToCreateSegments = SegmentEditorAPI::getInstance()->isUserCanAddNewSegment($this->idSite);
         $this->isUserAnonymous = Piwik::isUserIsAnonymous();
         $this->segmentTranslations = $this->getTranslations();
+        $this->segmentProcessedOnRequest = Rules::isBrowserArchivingAvailableForSegments();
+        $this->hideSegmentDefinitionChangeMessage = UsersManagerAPI::getInstance()->getUserPreference(Piwik::getCurrentUserLogin(), 'hideSegmentDefinitionChangeMessage');
     }
 
     public function getClientSideProperties()
diff --git a/plugins/SegmentEditor/javascripts/Segmentation.js b/plugins/SegmentEditor/javascripts/Segmentation.js
index ce3c166cf1..7365a89fd6 100644
--- a/plugins/SegmentEditor/javascripts/Segmentation.js
+++ b/plugins/SegmentEditor/javascripts/Segmentation.js
@@ -1075,7 +1075,44 @@ Segmentation = (function($) {
                 jQuery.extend(params, {
                     "idSegment": segmentId
                 });
-                self.updateMethod(params);
+
+                if(segmentStr != getSegmentFromId(segmentId).definition && $('.segment-definition-change-confirm').data('hideMessage') != 1) {
+                    var segmentProcessedOnRequest = $('.segment-definition-change-confirm').data('segmentProcessedOnRequest');
+
+                    $('.process-on-request, .no-process-on-request').hide();
+
+                    if (segmentProcessedOnRequest == 1 && autoArchive == 0) {
+                        $('.process-on-request').show();
+                    } else {
+                        $('.no-process-on-request').show();
+                    }
+
+                    piwikHelper.modalConfirm('.segment-definition-change-confirm', {
+                        yes: function () {
+                            if ($('#hideSegmentMessage:checked').length) {
+                                var ajaxHandler = new ajaxHelper();
+                                ajaxHandler.setLoadingElement();
+                                ajaxHandler.addParams({
+                                    "module": 'API',
+                                    "format": 'json',
+                                    "method": 'UsersManager.setUserPreference',
+                                    "userLogin": piwik.userLogin,
+                                    "preferenceName": "hideSegmentDefinitionChangeMessage",
+                                    "preferenceValue": "1"
+                                }, 'GET');
+                                ajaxHandler.useCallbackInCaseOfError();
+                                ajaxHandler.setCallback(function (response) {
+                                    self.updateMethod(params);
+                                });
+                                ajaxHandler.send(true);
+                            } else {
+                                self.updateMethod(params);
+                            }
+                        }
+                    });
+                } else {
+                    self.updateMethod(params);
+                }
             }
         };
 
diff --git a/plugins/SegmentEditor/lang/en.json b/plugins/SegmentEditor/lang/en.json
index 96e36c02b7..286682b813 100644
--- a/plugins/SegmentEditor/lang/en.json
+++ b/plugins/SegmentEditor/lang/en.json
@@ -6,11 +6,14 @@
         "AreYouSureDeleteSegment": "Are you sure you want to delete this segment?",
         "AutoArchivePreProcessed": "segmented reports are pre-processed (faster, requires cron)",
         "AutoArchiveRealTime": "segmented reports are processed in real time",
+        "ChangingSegmentDefinitionConfirmationNotProcessedOnRequest": "You're about to change the segment definition. Your analytics reports for this new segment won't be available until the reports are re-processed. It may take a few hours for reports data to show for this segment. Proceed anyway?",
+        "ChangingSegmentDefinitionConfirmationProcessedOnRequest": "You're about to change the segment definition. Your analytics reports for this new segment will be re-processed the next time you request them, which may take some time. Proceed anyway?",
         "ChooseASegment": "Choose a segment",
         "CurrentlySelectedSegment": "Currently selected segment: %s",
         "DataAvailableAtLaterDate": "Your segmented analytics reports will be available later. We apologize for the inconvenience.",
         "DefaultAllVisits": "All visits",
         "DragDropCondition": "Drag & Drop condition",
+        "HideMessageInFuture": "Hide this message in the future",
         "LoadingSegmentedDataMayTakeSomeTime": "Processing segmented visitor data may take a few minutes...",
         "OperatorAND": "AND",
         "OperatorOR": "OR",
diff --git a/plugins/SegmentEditor/stylesheets/segmentation.less b/plugins/SegmentEditor/stylesheets/segmentation.less
index afc79b3a9b..54367641a6 100644
--- a/plugins/SegmentEditor/stylesheets/segmentation.less
+++ b/plugins/SegmentEditor/stylesheets/segmentation.less
@@ -660,6 +660,17 @@ a.metric_category {
     box-shadow: 0 1px 2px 0 rgba(0,0,0,0.14), 0 3px 1px -2px rgba(0,0,0,0.2), 0 1px 5px 0 rgba(0,0,0,0.12);
 }
 
+.segment-definition-change-confirm .description {
+    margin-top: 20px;
+    float: left;
+    width: 100%;
+    text-align: center;
+
+    > span {
+        display: inline-block;
+    }
+}
+
 @media all and (max-width: 749px) {
     span.segmentationTitle,
     .segmentEditorPanel.expanded .segmentationContainer {
@@ -669,7 +680,7 @@ a.metric_category {
 
 .dropdown-body {
     background-color: #f7f7f7;
-    border-top-width: 0px;
+    border-top-width: 0;
     display: none;
     border-bottom-left-radius: 4px;
     border-bottom-right-radius: 4px;
diff --git a/plugins/SegmentEditor/templates/_segmentSelector.twig b/plugins/SegmentEditor/templates/_segmentSelector.twig
index 22241c389f..c4feb158c1 100644
--- a/plugins/SegmentEditor/templates/_segmentSelector.twig
+++ b/plugins/SegmentEditor/templates/_segmentSelector.twig
@@ -167,6 +167,24 @@
     <input role="yes" type="button" value="{{ 'General_Yes'|translate }}"/>
     <input role="no" type="button" value="{{ 'General_No'|translate }}"/>
 </div>
+<div class="ui-confirm segment-definition-change-confirm" data-segment-processed-on-request="{{ segmentProcessedOnRequest|number_format }}" data-hide-message="{{ hideSegmentDefinitionChangeMessage }}">
+    <h2>
+        <span class="process-on-request">
+            {{ 'SegmentEditor_ChangingSegmentDefinitionConfirmationProcessedOnRequest'|translate }}
+        </span>
+        <span class="no-process-on-request">
+            {{ 'SegmentEditor_ChangingSegmentDefinitionConfirmationNotProcessedOnRequest'|translate }}
+        </span>
+    </h2>
+    <p class="description">
+        <span>
+            <input type="checkbox" id="hideSegmentMessage" name="hideSegmentMessage" />
+            <label for="hideSegmentMessage">{{ 'SegmentEditor_HideMessageInFuture'|translate }}</label>
+        </span>
+    </p>
+    <input role="yes" type="button" value="{{ 'General_Yes'|translate }}"/>
+    <input role="no" type="button" value="{{ 'General_No'|translate }}"/>
+</div>
 <div class="ui-confirm pleaseChangeBrowserAchivingDisabledSetting">
     <h2>{{ 'SegmentEditor_SegmentNotApplied'|translate(nameOfCurrentSegment)|raw }}</h2>
     {% set segmentSetting %}{{ 'SegmentEditor_AutoArchivePreProcessed'|translate }}{% endset %}
diff --git a/tests/UI/specs/SegmentSelectorEditor_spec.js b/tests/UI/specs/SegmentSelectorEditor_spec.js
index c28c9e6cb4..0fec52330f 100644
--- a/tests/UI/specs/SegmentSelectorEditor_spec.js
+++ b/tests/UI/specs/SegmentSelectorEditor_spec.js
@@ -130,8 +130,8 @@ describe("SegmentSelectorEditorTest", function () {
         }, done);
     });
 
-    it("should correctly update the segment when its details are changed and saved", function (done) {
-        expect.screenshot("updated").to.be.captureSelector(selectorsToCapture, function (page) {
+    it("should correctly should show a confirmation when changing segment definition", function (done) {
+        expect.screenshot("update_confirmation").to.be.captureSelector('.ui-dialog', function (page) {
             page.click('.segmentEditorPanel .editSegmentName');
             page.evaluate(function () {
                 $('input.edit_segment_name').val('');
@@ -150,7 +150,12 @@ describe("SegmentSelectorEditorTest", function () {
             });
 
             page.click('button.saveAndApply');
+        }, done);
+    });
 
+    it("should correctly update the segment when saving confirmed", function (done) {
+        expect.screenshot("updated").to.be.captureSelector(selectorsToCapture, function (page) {
+            page.click('.ui-dialog button:contains(Yes)');
             page.click('.segmentationContainer');
         }, done);
     });