Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Technical review issues #2

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .php-version
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
8.2.20
4 changes: 1 addition & 3 deletions .phpcs.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<exclude-pattern>vendor</exclude-pattern>
<exclude-pattern>node_modules</exclude-pattern>
<exclude-pattern>registration.php</exclude-pattern>
<exclude-pattern>tmp</exclude-pattern>

<arg value="ps"/>
<arg name="colors"/>
Expand All @@ -14,9 +15,6 @@

<rule ref="Magento2">
<exclude name="PSR12.Properties.ConstantVisibility.NotFound" />

<!-- TODO: Upgrade obsolete migration scripts to Declarative Schema and Data Patches -->
<exclude name="Magento2.Legacy.InstallUpgrade" />
</rule>
<rule ref="PHPCompatibility"/>
</ruleset>
18 changes: 16 additions & 2 deletions Block/Adminhtml/SelectWidgetBlock.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

namespace Tawk\Widget\Block\Adminhtml;

use Magento\Framework\Escaper;
use Magento\Backend\Block\Template;
use Tawk\Widget\Model\WidgetFactory;

Expand Down Expand Up @@ -51,19 +52,32 @@ class SelectWidgetBlock extends Template
*/
private static $validPatternList;

/**
* Escaper instance
*
* @var Escaper $escaper
*/
public $escaper;
eug-L marked this conversation as resolved.
Show resolved Hide resolved

/**
* Constructor
*
* @param Template\Context $context Template context
* @param WidgetFactory $modelFactory Tawk.to Widget Model instance
* @param Escaper $escaper Escaper instance
* @param array $data Template data
*/
public function __construct(Template\Context $context, WidgetFactory $modelFactory, array $data = [])
{
public function __construct(
Template\Context $context,
WidgetFactory $modelFactory,
Escaper $escaper,
array $data = []
) {
parent::__construct($context, $data);
$this->logger = $context->getLogger();
$this->modelWidgetFactory = $modelFactory;
$this->request = $context->getRequest();
$this->escaper = $escaper;
}

/**
Expand Down
15 changes: 13 additions & 2 deletions Block/Embed.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
namespace Tawk\Widget\Block;

use Magento\Framework\View\Element\Template;
use Magento\Framework\Escaper;
eug-L marked this conversation as resolved.
Show resolved Hide resolved
use Magento\Customer\Model\SessionFactory;

use Tawk\Modules\UrlPatternMatcher;
Expand Down Expand Up @@ -68,18 +69,27 @@ class Embed extends Template
*/
protected $modelSessionFactory;

/**
* Escaper instance
*
* @var Escaper $escaper
*/
public $escaper;

/**
* Constructor
*
* @param SessionFactory $sessionFactory Session Factory instance
* @param WidgetFactory $modelFactory Tawk.to Widget Model instance
* @param Template\Context $context Template Context
* @param Escaper $escaper Escaper instance
* @param array $data Template data
*/
public function __construct(
SessionFactory $sessionFactory,
WidgetFactory $modelFactory,
Template\Context $context,
Escaper $escaper,
array $data = []
) {
parent::__construct($context, $data);
Expand All @@ -89,6 +99,7 @@ public function __construct(
$this->model = $this->getWidgetModel();
$this->request = $context->getRequest();
$this->modelSessionFactory = $sessionFactory->create();
$this->escaper = $escaper;
}

/**
Expand All @@ -99,8 +110,8 @@ public function __construct(
public function getEmbedUrl()
{
return 'https://embed.tawk.to'.
'/'.htmlspecialchars($this->model->getPageId()).
'/'.htmlspecialchars($this->model->getWidgetId());
'/'.$this->escaper->escapeHtml($this->model->getPageId()).
'/'.$this->escaper->escapeHtml($this->model->getWidgetId());
eug-L marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
103 changes: 0 additions & 103 deletions Setup/InstallSchema.php

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,85 +1,99 @@
<?php

namespace Tawk\Widget\Setup;
declare(strict_types=1);

namespace Tawk\Widget\Setup\Patch\Data;

use Magento\Framework\Setup\UpgradeDataInterface;
use Magento\Framework\Setup\ModuleDataSetupInterface;
use Magento\Framework\Setup\ModuleContextInterface;
use Magento\Framework\Setup\Patch\DataPatchInterface;
use Magento\Store\Model\StoreManagerInterface;
use Laminas\Uri\UriFactory;

use Tawk\Helpers\PathHelper;
use Tawk\Widget\Model\WidgetFactory;

class UpgradeData implements UpgradeDataInterface
class UpdateExcludeIncludeUrl implements DataPatchInterface
{
/**
* Tawk.to Widget Model instance
*
* @var WidgetFactory $_modelWidgetFactory
*/
protected $_modelWidgetFactory;
private $_modelWidgetFactory;

/**
* Store Manager instance
*
* @var StoreManagerInterface $_modelStoreManager
*/
protected $_modelStoreManager;
private $_modelStoreManager;

/**
* Module Data Setup Interface
*
* @var ModuleDataSetupInterface
*/
private $_moduleDataSetup;

/**
* Constructor
*
* @param WidgetFactory $modelWidgetFactory Tawk.to Widget Model instance
* @param StoreManagerInterface $modelStoreManager Store Manager instance
* @param ModuleDataSetupInterface $moduleDataSetup Module Data Setup Interface
*/
public function __construct(
WidgetFactory $modelWidgetFactory,
StoreManagerInterface $modelStoreManager
StoreManagerInterface $modelStoreManager,
ModuleDataSetupInterface $moduleDataSetup
) {
$this->_modelWidgetFactory = $modelWidgetFactory;
$this->_modelStoreManager = $modelStoreManager;
$this->_moduleDataSetup = $moduleDataSetup;
}

/**
* Upgrade runner
* Get aliases (previous names) for the patch.
*
* @param ModuleDataSetupInterface $setup Module Data Setup instance
* @param ModuleContextInterface $context Module Context Setup instance
* @return void
* @return string[]
*/
public function upgrade(ModuleDataSetupInterface $setup, ModuleContextInterface $context)
public function getAliases()
{
$setup->startSetup();
$this->versionUpdate160($setup, $context);
$setup->endSetup();
return [];
}

/**
* Upgrade script for version 1.6.0
* Get array of patches that have to be executed prior to this.
*
* @return string[]
*/
public static function getDependencies()
{
return [];
}

/**
* Add new records with wildcards that are derived from the existing patterns.
*
* @param [type] $setup
* @param [type] $context
* @return void
*/
private function versionUpdate160($setup, $context)
public function apply()
{
if (version_compare($context->getVersion(), '1.6.0', '<')) {
// get all stores and groups
$collection = $this->_modelWidgetFactory->create()->getCollection();

foreach ($collection as $item) {
$storeId = $item->getForStoreId();
$storeHost = $this->getStoreHost($storeId);
$excludePatternList = $this->addWildcardToPatternList($item->getExcludeUrl(), $storeHost);
$includePatternList = $this->addWildcardToPatternList($item->getIncludeUrl(), $storeHost);

$item->setExcludeUrl($excludePatternList);
$item->setIncludeUrl($includePatternList);
$item->save();
}
$this->_moduleDataSetup->getConnection()->startSetup();

$collection = $this->_modelWidgetFactory->create()->getCollection();

foreach ($collection as $item) {
$storeId = $item->getStoreId();
$storeHost = $this->getStoreHost($storeId);

$excludePatternList = $this->addWildcardToPatternList($item->getExcludeUrl(), $storeHost);
$includePatternList = $this->addWildcardToPatternList($item->getIncludeUrl(), $storeHost);

$item->setExcludeUrl($excludePatternList);
$item->setIncludeUrl($includePatternList);
$item->save();
}

$this->_moduleDataSetup->getConnection()->endSetup();
}

/**
Expand All @@ -93,15 +107,14 @@ private function getStoreHost($storeId)
$storeHost = '';

$storeUrl = $this->_modelStoreManager->getStore($storeId)->getBaseUrl();
//phpcs:ignore Magento2.Functions.DiscouragedFunction.Discouraged
$parsedUrl = parse_url($storeUrl);
$parsedUrl = UriFactory::factory($storeUrl);

if (!empty($parsedUrl['host'])) {
$storeHost = $parsedUrl['host'];
if (!empty($parsedUrl->getHost())) {
$storeHost = $parsedUrl->getHost();
}

if (!empty($parsedUrl['port'])) {
$storeHost .= ':' . $parsedUrl['port'];
if (!empty($parsedUrl->getPort())) {
$storeHost .= ':' . $parsedUrl->getPort();
}

return $storeHost;
Expand Down
Loading