Skip to content

Added log when currency rates import fail #3616

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

Added log when currency rates import fail #3616

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cdd0a6a
Log exception when importing currencies
kyrena Oct 27, 2023
fbbddf4
Remove unused translations
kyrena Oct 27, 2023
99c85db
Remove $importModel override
kyrena Oct 27, 2023
826c418
Declare $importModel
kyrena Oct 27, 2023
4d6b64f
Check $rates
kyrena Oct 27, 2023
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
58 changes: 33 additions & 25 deletions app/code/core/Mage/Directory/Model/Observer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,49 +22,57 @@
class Mage_Directory_Model_Observer
{
public const CRON_STRING_PATH = 'crontab/jobs/currency_rates_update/schedule/cron_expr';
public const IMPORT_ENABLE = 'currency/import/enabled';
public const IMPORT_SERVICE = 'currency/import/service';
public const IMPORT_ENABLE = 'currency/import/enabled';
public const IMPORT_SERVICE = 'currency/import/service';

public const XML_PATH_ERROR_TEMPLATE = 'currency/import/error_email_template';
public const XML_PATH_ERROR_IDENTITY = 'currency/import/error_email_identity';
public const XML_PATH_ERROR_TEMPLATE = 'currency/import/error_email_template';
public const XML_PATH_ERROR_IDENTITY = 'currency/import/error_email_identity';
public const XML_PATH_ERROR_RECIPIENT = 'currency/import/error_email';

/**
* @param Mage_Cron_Model_Schedule|null $cron
* @throws Mage_Core_Exception
*/
public function scheduledUpdateCurrencyRates()
public function scheduledUpdateCurrencyRates($cron = null)
Copy link
Contributor

@sreichel sreichel Oct 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pls dont change signatures of used methods. Better implement new ones.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about this?

if (func_num_args() && func_get_args()[0] instanceof Mage_Cron_Model_Schedule) {
    $schedule = func_get_args()[0];
    //....
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add a new method w/ parameter and all logic in and forward to this method. Similiar i did with #2940.

{
$importWarnings = [];
if (!Mage::getStoreConfig(self::IMPORT_ENABLE) || !Mage::getStoreConfig(self::CRON_STRING_PATH)) {
return;
}

$service = Mage::getStoreConfig(self::IMPORT_SERVICE);
if (!$service) {
$importWarnings[] = Mage::helper('directory')->__('FATAL ERROR:') . ' ' . Mage::helper('directory')->__('Invalid Import Service specified.');
}
$errors = [];
$service = (string)Mage::getStoreConfig(self::IMPORT_SERVICE);
$importModel = null;

try {
/** @var Mage_Directory_Model_Currency_Import_Abstract $importModel */
$importModel = Mage::getModel(Mage::getConfig()->getNode('global/currency/import/services/' . $service . '/model')->asArray());
} catch (Exception $e) {
$importWarnings[] = Mage::helper('directory')->__('FATAL ERROR:') . ' ' . Mage::throwException(Mage::helper('directory')->__('Unable to initialize the import model.'));
if ($service) {
try {
/** @var Mage_Directory_Model_Currency_Import_Abstract $importModel */
$importModel = Mage::getModel(Mage::getConfig()->getNode('global/currency/import/services/' . $service . '/model')->asArray());
} catch (Exception $e) {
$errors[] = 'FATAL ERROR: Unable to initialize the import model (' . $e->getMessage() . ').';
}
} else {
$errors[] = 'FATAL ERROR: Invalid Import Service specified (' . $service . ').';
}

$rates = $importModel->fetchRates();
$errors = $importModel->getMessages();

if (count($errors)) {
foreach ($errors as $error) {
$importWarnings[] = Mage::helper('directory')->__('WARNING:') . ' ' . $error;
}
if (is_object($importModel)) {
$rates = $importModel->fetchRates();
$errors = $importModel->getMessages();
}

if (!count($importWarnings)) {
if (isset($rates) && !count($errors)) {
Mage::getModel('directory/currency')->saveRates($rates);
} else {
$translate = Mage::getSingleton('core/translate');
$errors = 'An error occured while importing currency rates, no rates updated.' . "\n- " . implode("\n- ", $errors);

Mage::logException(new Exception($errors));
if (is_object($cron)) {
/** @var Mage_Cron_Model_Schedule $cron */
$cron->setMessages($errors);
$cron->setIsError(true);
}

/** @var Mage_Core_Model_Translate $translate */
$translate = Mage::getSingleton('core/translate');
$translate->setTranslateInline(false);

/** @var Mage_Core_Model_Email_Template $mailTemplate */
Expand All @@ -77,7 +85,7 @@ public function scheduledUpdateCurrencyRates()
Mage::getStoreConfig(self::XML_PATH_ERROR_RECIPIENT),
null,
[
'warnings' => implode("\n", $importWarnings),
'warnings' => $errors,
]
);

Expand Down
3 changes: 0 additions & 3 deletions app/locale/en_US/Mage_Directory.csv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
"Error Email Recipient","Error Email Recipient"
"Error Email Sender","Error Email Sender"
"Error Email Template","Error Email Template"
"FATAL ERROR:","FATAL ERROR:"
"Fixer.IO","Fixer.IO"
"Frequency","Frequency"
"Installed Currencies","Installed Currencies"
Expand All @@ -52,9 +51,7 @@
"The account this API request is coming from is inactive.","The account this API request is coming from is inactive."
"The current request did not return any results.","The current request did not return any results."
"The maximum allowed API amount of monthly API requests has been reached.","The maximum allowed API amount of monthly API requests has been reached."
"Unable to initialize the import model.","Unable to initialize the import model."
"Undefined rate from ""%s-%s"".","Undefined rate from ""%s-%s""."
"WARNING:","WARNING:"
"We can't retrieve a rate from %s for %s.","We can't retrieve a rate from %s for %s."
"Webservicex","Webservicex"
"Your Currency","Your Currency"
Expand Down
5 changes: 0 additions & 5 deletions phpstan.dist.baseline.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3020,11 +3020,6 @@ parameters:
count: 1
path: app/code/core/Mage/Directory/Model/Currency/Import/Webservicex.php

-
message: "#^Variable \\$importModel might not be defined\\.$#"
count: 2
path: app/code/core/Mage/Directory/Model/Observer.php

-
message: "#^Left side of && is always true\\.$#"
count: 1
Expand Down