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

Prestashop module review - Edge case improvements #121

Merged
merged 9 commits into from
Apr 8, 2024
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
"prestashop/ps_facetedsearch": "^3.7.1",
"prestashop/ps_faviconnotificationbo": "^2",
"prestashop/ps_featuredproducts": "^2",
"prestashop/ps_googleanalytics": "^4.0",
"prestashop/ps_googleanalytics": "^5.0",
"prestashop/ps_imageslider": "^3",
"prestashop/ps_languageselector": "^2",
"prestashop/ps_linklist": "^6",
Expand Down
365 changes: 180 additions & 185 deletions composer.lock

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ services:
BTCPAY_BTCEXPLORERURL: 'http://nbxplorer:32838/'
expose:
- '49392'
image: 'btcpayserver/btcpayserver:1.12.5'
image: 'btcpayserver/btcpayserver:1.13.0'
links:
- postgres
ports:
Expand Down Expand Up @@ -71,7 +71,7 @@ services:
NBXPLORER_BTCNODEENDPOINT: 'bitcoind:39388'
expose:
- '32838'
image: 'nicolasdorier/nbxplorer:2.5.0'
image: 'nicolasdorier/nbxplorer:2.5.2'
links:
- postgres
volumes:
Expand Down
33 changes: 27 additions & 6 deletions modules/btcpay/btcpay.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public function __construct()
{
$this->name = 'btcpay';
$this->tab = 'payments_gateways';
$this->version = '6.1.3';
$this->version = '6.1.4';
$this->author = 'BTCPay Server';
$this->ps_versions_compliancy = ['min' => Constants::MINIMUM_PS_VERSION, 'max' => _PS_VERSION_];
$this->controllers = ['payment', 'validation', 'webhook'];
Expand Down Expand Up @@ -490,11 +490,32 @@ private function displayModuleWarnings(): void
// Try and create the client
$client = Client::createFromConfiguration($this->configuration);

// Show warning if API key is missing or if we're not yet fully synced
if (null === $client || empty($this->configuration->get(Constants::CONFIGURATION_BTCPAY_API_KEY))) {
$this->warning = $this->trans('Your BTCPay Server store has not yet been linked, payment option is unavailable.', [], 'Modules.Btcpay.Admin');
} elseif (!$client->server()->getInfo()->isFullySynced()) {
$this->warning = $this->trans('One (or more) coins are not yet synced, payment option will be unavailable until the sync has finished.', [], 'Modules.Btcpay.Admin');
try {
// Show warning if API key is missing or if we're not yet fully synced
if (null === $client || empty($this->configuration->get(Constants::CONFIGURATION_BTCPAY_API_KEY))) {
$this->warning = $this->trans('Your BTCPay Server store has not yet been linked, payment option is unavailable.', [], 'Modules.Btcpay.Admin');
} elseif (!$client->server()->getInfo()->isFullySynced()) {
$this->warning = $this->trans('One (or more) coins are not yet synced, payment option will be unavailable until the sync has finished.', [], 'Modules.Btcpay.Admin');
}
} catch (BTCPayException $exception) {
// Log the exception
PrestaShopLogger::addLog(\sprintf('[WARNING] BTCPay Server configuration is no longer valid, resetting host and API key. Error: %s', $exception->getMessage()), \PrestaShopLogger::LOG_SEVERITY_LEVEL_WARNING, $exception->getCode());

// Show a warning
$this->warning = $this->trans('Your BTCPay Server configuration is no longer valid, please setup module again.', [], 'Modules.Btcpay.Admin');

// Reset configuration
$this->configuration->set(Constants::CONFIGURATION_BTCPAY_HOST, Constants::CONFIGURATION_DEFAULT_HOST);
$this->configuration->set(Constants::CONFIGURATION_BTCPAY_API_KEY, null);

// Bail
return;
} catch (\Throwable $exception) {
// Log the exception
PrestaShopLogger::addLog(\sprintf('[WARNING] Could not parse server information: %s', $exception->getMessage()), \PrestaShopLogger::LOG_SEVERITY_LEVEL_WARNING, $exception->getCode());

// Bail
return;
}

// API key/sync warnings are more important than a new version, if a warning is set, return now
Expand Down
157 changes: 81 additions & 76 deletions modules/btcpay/composer.lock

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Validator\ConstraintViolationInterface;
use Symfony\Component\Validator\Validator\ValidatorInterface;
use Throwable;

Expand Down Expand Up @@ -225,16 +226,23 @@ public function validateAPIKeyAction(Request $request): Response
if (empty($request->request->all())) {
$this->addFlash('error', 'Did not receive data from BTCPay Server. If you received an <strong>Invalid Token</strong> page, make sure to properly setup PrestaShop and BTCPay Server (publicly accessible and HTTPS enabled). Please try again once done or use the API key option.');

// Make sure to reset the API key
$this->getConfiguration()->set(Constants::CONFIGURATION_BTCPAY_API_KEY, null);

return $this->redirectToRoute('admin_btcpay_configure');
}

// Validate incoming request and return any errors we encounter
$validateRequest = new ValidateApiKey($request->request);
if (0 !== \count($errors = $this->validator->validate($validateRequest))) {
/** @var ConstraintViolationInterface $error */
foreach ($errors as $error) {
$this->addFlash('error', $error->getMessage());
}

// Make sure to reset the API key
$this->getConfiguration()->set(Constants::CONFIGURATION_BTCPAY_API_KEY, null);

return $this->redirectToRoute('admin_btcpay_configure');
}

Expand Down Expand Up @@ -300,16 +308,23 @@ private function processApiKey(Server $configuration): RedirectResponse
// Validate created API key and return any errors we encounter
$validateKey = new ValidateApiKey(new ParameterBag($client->apiKey()->getCurrent()->getData()));
if (0 !== \count($errors = $this->validator->validate($validateKey))) {
/** @var ConstraintViolationInterface $error */
foreach ($errors as $error) {
$this->addFlash('error', $error->getMessage());
}

// Make sure to reset the API key
$shopConfiguration->set(Constants::CONFIGURATION_BTCPAY_API_KEY, null);

return $this->redirectToRoute('admin_btcpay_configure');
}

// Get the store ID
$storeId = $validateKey->getStoreID();

// Grab the store
$store = $client->store()->getStore($storeId);

// Ensure we have a valid BTCPay Server version
if (null !== ($info = $client->server()->getInfo()) && \version_compare($info->getVersion(), Constants::MINIMUM_BTCPAY_VERSION, '<')) {
$this->addFlash('error', \sprintf('BTCPay server version is too low. Expected %s or higher, received %s.', Constants::MINIMUM_BTCPAY_VERSION, $info->getVersion()));
Expand All @@ -323,8 +338,8 @@ private function processApiKey(Server $configuration): RedirectResponse

// Ensure we have a payment methods setup
if (empty($client->payment()->getPaymentMethods($storeId))) {
$this->addFlash('error', \sprintf("This plugin expects a payment method to have been setup for store '%s'.", $client->store()->getStore($storeId)->offsetGet('name')));
PrestaShopLogger::addLog(\sprintf("[ERROR] This plugin expects a payment method to have been setup for store '%s'.", $client->store()->getStore($storeId)->offsetGet('name')), PrestaShopLogger::LOG_SEVERITY_LEVEL_ERROR);
$this->addFlash('error', \sprintf("This plugin expects a payment method to have been setup for store '%s'.", $store->offsetGet('name')));
PrestaShopLogger::addLog(\sprintf("[ERROR] This plugin expects a payment method to have been setup for store '%s'.", $store->offsetGet('name')), PrestaShopLogger::LOG_SEVERITY_LEVEL_ERROR);

// Make sure to reset the API key
$shopConfiguration->set(Constants::CONFIGURATION_BTCPAY_API_KEY, null);
Expand Down
4 changes: 2 additions & 2 deletions modules/btcpay/src/Server/Data/ValidateApiKey.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public function getStoreID(): string
}

/**
* @Assert\IsTrue(message="This plugin expects all passed permissions to be given. Remove the new API key and try again.")
* @Assert\IsTrue(message="This plugin expects all passed permissions to be given. Please remove and recreate the API key")
*/
public function hasRequiredPermissions(): bool
{
Expand All @@ -59,7 +59,7 @@ public function hasRequiredPermissions(): bool
}

/**
* @Assert\IsTrue(message="This plugin requires one store (and one store only) to be authorized. Remove the new API key and try again.")
* @Assert\IsTrue(message="This plugin requires one store (and one store only) to be authorized. Please remove and recreate the API key.")
*/
public function hasSingleStore(): bool
{
Expand Down
3 changes: 2 additions & 1 deletion modules/btcpay/views/templates/admin/configure.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@
</div>

<div class="card-footer text-right">
<a href="{{ server_form.vars.value.host }}" class="btn btn-primary" target="_blank" rel="noopener noreferrer nofollow">{{ 'Visit Dashboard'|trans({}, 'Admin.Actions') }}</a>
<a href="{{ authorizeUrl }}" class="btn btn-primary" target="_blank" rel="noopener noreferrer nofollow">{{ 'Create API key'|trans({}, 'Admin.Actions') }}</a>
</div>
</div>
Expand All @@ -142,7 +143,7 @@
<div class="card-text">
<dl>
<dt><span class="text-muted mb-0"><strong>{{ 'Linked Store'|trans({}, 'Modules.Btcpay.Admin') }}</strong></span></dt>
<dd><span class="px-1"><a href="{{ client.baseUrl }}" target="_blank" rel="noopener noreferrer nofollow">{{ storeInfo.name }}</a></span></dd>
<dd><span class="px-1"><a href="{{ client.baseUrl }}/stores/{{ storeInfo.id }}" target="_blank" rel="noopener noreferrer nofollow">{{ storeInfo.name }}</a></span></dd>

<dt><span class="text-muted mb-0"><strong>{{ 'Default speed policy'|trans({}, 'Modules.Btcpay.Admin') }}</strong></span></dt>
<dd><span class="px-1">{{ storeInfo.speedPolicy }}</span></dd>
Expand Down