Merge pull request #3 from Magenerds/feature/more_informative_responses

Feature/more informative responses
Magenerds · Oct 5, 2017 · 0df7313 · 0df7313
2 parents ec79298 + 238ee5f
commit 0df7313
Show file tree

Hide file tree

Showing 11 changed files with 331 additions and 46 deletions.
diff --git a/Block/System/Config/Form/SyncButton.php b/Block/System/Config/Form/SyncButton.php
@@ -9,6 +9,7 @@
 
 namespace Magenerds\SystemDiff\Block\System\Config\Form;
 
+use Magenerds\SystemDiff\Helper\Config;
 use Magento\Config\Block\System\Config\Form\Field;
 use Magento\Framework\Data\Form\Element\AbstractElement;
 
@@ -24,22 +25,49 @@ class SyncButton extends Field
      */
     protected $element;
 
+    /**
+     * @var Config
+     */
+    public $configHelper;
+
+    /**
+     * SyncButton constructor.
+     *
+     * Additionally injects config helper.
+     *
+     * @param \Magento\Backend\Block\Template\Context $context
+     * @param array                                   $data
+     * @param Config                                  $configHelper
+     */
+    public function __construct(
+        \Magento\Backend\Block\Template\Context $context,
+        Config $configHelper,
+        array $data = []
+    ) {
+        $this->configHelper = $configHelper;
+
+        parent::__construct($context, $data);
+    }
+
     /**
      * Remove scope label
      *
      * @param  AbstractElement $element
+     *
      * @return string
      */
     public function render(AbstractElement $element)
     {
         $element->unsScope()->unsCanUseWebsiteValue()->unsCanUseDefaultValue();
+
         return parent::render($element);
     }
 
     /**
      * Return element html
      *
      * @param  AbstractElement $element
+     *
      * @return string
      */
     protected function _getElementHtml(AbstractElement $element)
@@ -73,7 +101,7 @@ public function getButtonHtml()
         );
         $button->setData(
             [
-                'id' => $this->element->getHtmlId(),
+                'id'    => $this->element->getHtmlId(),
                 'label' => __('Run'),
                 'class' => 'disabled' // reset when page loaded, see template script
             ]
@@ -82,6 +110,24 @@ public function getButtonHtml()
         return $button->toHtml();
     }
 
+    /**
+     * Returns info about last sync date time as a string for usage in "'"-JS string.
+     *
+     * @return string
+     */
+    public function getLastSyncInfo()
+    {
+        $formattedDateTime = $this->configHelper->getLastSyncDatetimeFormatted();
+
+        return (string)filter_var(
+            sprintf(
+                __('Last diff on %s'),
+                $formattedDateTime
+            ),
+            FILTER_SANITIZE_MAGIC_QUOTES
+        );
+    }
+
     /**
      * Returns the element defined in system.xml
      *
@@ -91,4 +137,4 @@ public function getElement()
     {
         return $this->element;
     }
-}
+}
diff --git a/Console/Command/ExecuteCommand.php b/Console/Command/ExecuteCommand.php
@@ -9,6 +9,7 @@
 
 namespace Magenerds\SystemDiff\Console\Command;
 
+use Magenerds\SystemDiff\Helper\Config;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
@@ -26,20 +27,34 @@ class ExecuteCommand extends Command
      */
     const COMMAND_DESCRIPTION = 'system-diff:execute';
 
+    /**
+     * Exit code when exception occurred
+     */
+    const EXIT_CODE_EXCEPTION = 4;
+
     /**
      * @var PerformSystemDiffService
      */
     private $performSystemDiffService;
 
+    /**
+     * @var Config
+     */
+    private $configHelper;
+
     /**
      * ExecuteCommand constructor.
+     *
      * @param PerformSystemDiffService $performSystemDiffService
+     * @param Config                   $configHelper
      */
     public function __construct(
-        PerformSystemDiffService $performSystemDiffService
-    ){
+        PerformSystemDiffService $performSystemDiffService,
+        Config $configHelper
+    ) {
         parent::__construct(self::COMMAND_NAME);
         $this->performSystemDiffService = $performSystemDiffService;
+        $this->configHelper = $configHelper;
     }
 
     /**
@@ -54,16 +69,28 @@ public function configure()
     }
 
     /**
-     * @param InputInterface $input An InputInterface instance
+     * @param InputInterface  $input  An InputInterface instance
      * @param OutputInterface $output An OutputInterface instance
-     * @return null|int null or 0 if everything went fine, or an error code
+     *
+     * @return int 0 if everything went fine, or an error code
      */
     public function execute(InputInterface $input, OutputInterface $output)
     {
+        $exitStatus = 0;
         try {
+            $output->write('Performing sync and diff...');
             $this->performSystemDiffService->performDiff();
+            $output->writeln(
+                sprintf(
+                    'Done at %s.',
+                    $this->configHelper->getLastSyncDatetimeFormatted()
+                )
+            );
         } catch (\Exception $e) {
+            $exitStatus = self::EXIT_CODE_EXCEPTION;
             $output->writeln(sprintf('An error occurred during diff: %s', $e->getMessage()));
         }
+
+        return $exitStatus;
     }
 }
diff --git a/Controller/Adminhtml/SystemDiff/Diff.php b/Controller/Adminhtml/SystemDiff/Diff.php
@@ -9,6 +9,7 @@
 
 namespace Magenerds\SystemDiff\Controller\Adminhtml\SystemDiff;
 
+use Magenerds\SystemDiff\Helper\Config;
 use Magenerds\SystemDiff\Service\PerformSystemDiffService;
 use Magento\Backend\App\Action;
 use Magento\Backend\App\Action\Context;
@@ -31,21 +32,29 @@ class Diff extends Action
      */
     private $performSystemDiffService;
 
+    /**
+     * @var Config
+     */
+    private $configHelper;
+
     /**
      * Diff action constructor.
      *
-     * @param Context $context
-     * @param JsonFactory $jsonFactory
+     * @param Context                  $context
+     * @param JsonFactory              $jsonFactory
      * @param PerformSystemDiffService $performSystemDiffService
+     * @param Config                   $configHelper
      */
     public function __construct(
         Context $context,
         JsonFactory $jsonFactory,
-        PerformSystemDiffService $performSystemDiffService
+        PerformSystemDiffService $performSystemDiffService,
+        Config $configHelper
     ) {
         $this->context = $context;
         $this->jsonFactory = $jsonFactory;
         $this->performSystemDiffService = $performSystemDiffService;
+        $this->configHelper = $configHelper;
 
         parent::__construct($context);
     }
@@ -60,15 +69,23 @@ public function execute()
 
         $result = $this->jsonFactory->create();
 
-        $message = 'OK';
+        $message = 'Diff successfully done at %s.';
 
         try {
             $this->performSystemDiffService->performDiff();
         } catch (\Exception $e) {
-            $message = "Error performing system diff.";
+            $message = __("Error performing system diff at %s.");
+            $result->setStatusHeader(500);
         }
 
-        $result->setData(['message' => $message]);
+        $result->setData(
+            [
+                'message' => sprintf(
+                    __($message),
+                    $this->configHelper->getLastSyncDatetimeFormatted()
+                ),
+            ]
+        );
 
         return $result;
     }

diff --git a/DataWriter/StoreConfigDataWriter.php b/DataWriter/StoreConfigDataWriter.php
@@ -16,17 +16,6 @@
 
 class StoreConfigDataWriter implements DataWriterInterface
 {
-    /**
-     * Holds database field names
-     */
-    const LOCAL_VALUE_FIELD_NAME = 'diff_value_local';
-    const REMOTE_VALUE_FIELD_NAME = 'diff_value_remote';
-    const SCOPE_FIELD_NAME = 'scope';
-    const SCOPE_ID_FIELD_NAME = 'scope_id';
-    const PATH_FIELD_NAME = 'path';
-    const SCOPE_VALUE_WEBSITES = 'websites';
-    const SCOPE_VALUE_STORES = 'stores';
-
     /**
      * Holds further necessary consts
      */
@@ -101,17 +90,17 @@ public function write(array $diffData)
             foreach ($localValues as $localPath => $localValue) {
                 if (array_key_exists($localPath, $remoteValues)) {
                     $combinedValues[$localPath] = [
-                        self::LOCAL_VALUE_FIELD_NAME => $localValue,
-                        self::REMOTE_VALUE_FIELD_NAME => $remoteValues[$localPath]
+                        DiffConfigResource::LOCAL_VALUE_FIELD_NAME => $localValue,
+                        DiffConfigResource::REMOTE_VALUE_FIELD_NAME => $remoteValues[$localPath]
                     ];
 
                     unset($localValues[$localPath]);
                     unset($remoteValues[$localPath]);
                 }
             }
 
-            $localModels = $this->mapDataToModels($localValues, $scope, self::LOCAL_VALUE_FIELD_NAME);
-            $remoteModels = $this->mapDataToModels($remoteValues, $scope, self::REMOTE_VALUE_FIELD_NAME);
+            $localModels = $this->mapDataToModels($localValues, $scope, DiffConfigResource::LOCAL_VALUE_FIELD_NAME);
+            $remoteModels = $this->mapDataToModels($remoteValues, $scope, DiffConfigResource::REMOTE_VALUE_FIELD_NAME);
             $combinedModels = $this->mapDataToModels($combinedValues, $scope);
 
             $scopeModels = array_merge($localModels, $remoteModels, $combinedModels);
@@ -144,11 +133,11 @@ protected function mapDataToModels(array $data, $scope, $valueField = null)
         foreach ($data as $path => $value) {
             /** @var DiffConfigModel $diffConfigModel */
             $diffConfigModel = $this->diffConfigFactory->create();
-            $diffConfigModel->setData(self::SCOPE_FIELD_NAME, $scope);
+            $diffConfigModel->setScope($scope);
 
             $scopeId = self::DEFAULT_SCOPE_ID;
 
-            if ($scope === self::SCOPE_VALUE_WEBSITES || $scope === self::SCOPE_VALUE_STORES) {
+            if ($scope === DiffConfigResource::SCOPE_VALUE_WEBSITES || $scope === DiffConfigResource::SCOPE_VALUE_STORES) {
                 if (empty($path)) {
                     continue;
                 }
@@ -162,17 +151,17 @@ protected function mapDataToModels(array $data, $scope, $valueField = null)
                 $code = $splittedPath[0];
                 $path = $splittedPath[1];
 
-                if ($scope === self::SCOPE_VALUE_WEBSITES) {
+                if ($scope === DiffConfigResource::SCOPE_VALUE_WEBSITES) {
                     $scopeId = $this->getWebsiteId($code);
                 }
 
-                if ($scope === self::SCOPE_VALUE_STORES) {
+                if ($scope === DiffConfigResource::SCOPE_VALUE_STORES) {
                     $scopeId = $this->getStoreId($code);
                 }
             }
 
-            $diffConfigModel->setData(self::SCOPE_ID_FIELD_NAME, $scopeId);
-            $diffConfigModel->setData(self::PATH_FIELD_NAME, $path);
+            $diffConfigModel->setScope($scopeId);
+            $diffConfigModel->setPath($path);
 
             if (is_array($value) && is_null($valueField)) {
                 foreach ($value as $fieldName => $v) {
@@ -182,7 +171,6 @@ protected function mapDataToModels(array $data, $scope, $valueField = null)
                 $diffConfigModel->setData($valueField, $value);
             }
 
-
             $models[] = $diffConfigModel;
         }