Introduzione:
Il mercato e-commerce basato su magento2 è vasto e ricco. La piattaforma si adatta a molte delle situazioni e delle esigenze dei business che vogliono vendere i propri prodotti e servizi online. Molte (forse tutte) le esigenze non coperte dal core della piattaforma sono risolte da moduli di terze parti sviluppati da aziende tech (Vedi Amasty, Mageworx, Aheadworks, Mageplaza, Magepal, etc … ) o direttamente da quelle aziende che, offrendo un proprio servizio digitale, necessitano di integrarlo con le varie piattaforme di e-commerce.
E’ il caso di sistemi di pagamento, di piattaforme crm, di erp ed oms, piattaforme di email marketing e chi più ne ha ne metta.
Spesso però i moduli vengono sviluppati male (fare un modulo per woocommerce, magento o prestashop richiede si la conoscenza di PHP ma anche dei pattern delle piattaforme stesse), non vengono manutenuti, o crescono di feature in feature a seguito di nuove richieste generando bugs e perdendo l’iniziale identità.
Questo articolo nasce con lo scopo di raccontare la mia esperienza con il modulo ufficiale magento2 di Sendinblue (se non conoscete questa realtà ecco il link) ed il processo di refactoring effettuato.
Ho conosciuto Sendinblue quando ero dipendente e l’azienda per cui lavoravo necessitava di una soluzione a basso costo per spedire massivamente email per conto di un cliente. Serviva una soluzione facile da integrare e che presentasse delle API interrogabili agilmente.
Non mi dispiacque la tecnologia ed il funzionamento, tanto che appena presi i primi clienti Magento2 valutai l’adozione di questo servizio come default per coloro i quali non avessero già un fornitore per l’email marketing. C’era già un modulo ufficiale magento2, andava testato ed analizzato, così cominciai a studiarlo.
Bugs, malfunzionamenti e l’inizio degli sviluppi:
Il modulo ufficiale non era all’altezza, me ne resi subito conto. Non era manutenuto ed aggiornato (Non lo è nemmeno alla data di questo articolo in quanto, mentre sto scrivendo, l’ultimo commit risale al 7 dicembre 2020) il modulo non funzionava e dava diversi errori 500, la gestione delle configurazioni non utilizzava system.xml di magento2 ed aveva un suo controller adminhtml dedicato, inoltre era pieno zeppo di vizi di forma (object manager sparati un po’ ovunque, classi sparse senza un senso con dimensioni abnormi, non si utilizzavano le SDK per PHP ma chiamate dirette in curl, ed altre fantastiche cose ahimé).
Riprendere in mano il modulo facendo un fork non aveva senso: Il modulo andava rifatto da zero. Si potevano tenere le logiche di fondo, qualche observer ma di base era da cestinare.
Deciso a perseguire il mio obiettivo di proporre Sendinblue ai clienti (il servizio ripeto è valido a differenza del modulo Magento2) ho cominciato a riscrivere il modulo e, cosa che consiglio a chiunque debba fare un’operazione simile, ho proceduto come segue:
1) Pulizia tutti gli errori di scrittura e di applicazione ai design pattern di Magento2 (utilizzo diretto degli object manager al posto della Dependency injection, controller adminhtml dedicato al posto del system.xml, codice ripetuto, classi di più di 1000 righe, fix degli errori 500).
2) Riorganizzazione del modulo: divisione delle responsabilità delle varie classi e spacchettamento in più estensioni separate.
Refactoring e pulizia(1):
Una delle cose che più spesso capita di vedere in moduli di terze parti sviluppati male è l’utilizzo massivo di object manager in linea nelle varie classi (o addirittura direttamente nei file phtml). Prendiamo in esempio un frammento del codice del modulo ufficiale Sendinblue alla data odierna:
<?php
/**
* @author Sendinblue plateform <contact@sendinblue.com>
* @copyright 2013-2014 Sendinblue
* URL: https:www.sendinblue.com
* Do not edit or add to this file if you wish to upgrade Sendinblue Magento plugin to newer
* versions in the future. If you wish to customize Sendinblue magento plugin for your
* needs then we can't provide a technical support.
**/
namespace Sendinblue\Sendinblue\Observer;
use Magento\Framework\Event\Observer;
use Magento\Framework\Event\ObserverInterface;
use Sendinblue\Sendinblue\Model;
/**
* Customer Observer Model
*/
class SibObserver implements ObserverInterface
{
public function execute(Observer $observer)
{
$objectManager=\Magento\Framework\App\ObjectManager::getInstance();$model=$objectManager->create('Sendinblue\Sendinblue\Model\SendinblueSib');
$customer = $observer->getEvent()->getData('customer');
$customerId = $customer->getId();
$email= $customer->getEmail();
$NlStatus = $model->checkNlStatus($email);
$apiKey = $model->getDbData('api_key');
$sibStatus = $model->syncSetting();
if ($NlStatus == 1 && $sibStatus == 1) {
$firstName = $customer->getFirstName();
$lastName = $customer->getLastName();
$storeView = $customer->getCreatedIn();
$storeId = $customer->getStoreId();
$updateDataInSib = [];
$localeLang = $model->getDbData('sendin_config_lang');
if (!empty($firstName)) {
if ($localeLang == 'fr') {
$updateDataInSib['PRENOM'] = $firstName;
} else {
$updateDataInSib['NAME'] = $firstName;
}
}
if (!empty($lastName)) {
if ($localeLang == 'fr') {
$updateDataInSib['NOM'] = $lastName;
} else {
$updateDataInSib['SURNAME'] = $lastName;
}
}
if (!empty($firstName)) {
$updateDataInSib['CLIENT'] = 1;
} else {
$updateDataInSib['CLIENT'] = 0;
}
if (!empty($storeId)) {
$updateDataInSib['STORE_ID'] = $storeId;
}
if (!empty($storeView)) {
$updateDataInSib['MAGENTO_LANG'] = $storeView;
}
$model->subscribeByruntime($email, $updateDataInSib);
$model->sendWsTemplateMail($email);
}
}
}Si nota subito all’inizio del metodo execute() l’utilizzo dell’object Manager per ricavare l’istanza di un model. La prima cosa da fare è quindi dichiarare il costruttore ed utilizzare la dependency injection per ricavare l’istanza. Questo per motivi estetici (ordine del codice e pulizia) in primis ma anche per evitare un calo delle performance dato dal fatto che ogni volta che eseguo un create attraverso l’object manager questo mi restituisce una nuova istanza della classe richiesta invece di attingere dalle istanze già create e presenti nello state (consiglio un articolo di Alessandro Ronchi sul blog di Bitbull per capire meglio quando usare o meno l’object manager).
Quindi il codice sarebbe dovuto essere qualcosa del genere per utilizzare correttamente la dependency injection:
<?php
/**
* @author Sendinblue plateform <contact@sendinblue.com>
* @copyright 2013-2014 Sendinblue
* URL: https:www.sendinblue.com
* Do not edit or add to this file if you wish to upgrade Sendinblue Magento plugin to newer
* versions in the future. If you wish to customize Sendinblue magento plugin for your
* needs then we can't provide a technical support.
**/
namespace Sendinblue\Sendinblue\Observer;
use Magento\Framework\Event\Observer;
use Magento\Framework\Event\ObserverInterface;
use Sendinblue\Sendinblue\Model\SendinblueSib;
/**
* Class SibObserver
* @package Sendinblue\Sendinblue\Observer
*/
class SibObserver implements ObserverInterface
{
/**
* @var SendinblueSib
*/
protected $sibModel;/**
* SibObserver constructor.
* @param SendinblueSib $sibModel
*/
public function __construct(
SendinblueSib $sibModel
)
{
$this->sibModel = $sibModel;
}
/**
* @param Observer $observer
*/
public function execute(Observer $observer)
{
$customer = $observer->getEvent()->getData('customer');
$email = $customer->getEmail();
$NlStatus = $this->sibModel->checkNlStatus($email);
$sibStatus = $this->sibModel->syncSetting();
if ($NlStatus == 1 && $sibStatus == 1) {
$firstName = $customer->getFirstName();
$lastName = $customer->getLastName();
$storeView = $customer->getCreatedIn();
$storeId = $customer->getStoreId();
$updateDataInSib = [];
$localeLang = $this->sibModel->getDbData('sendin_config_lang');
if (!empty($firstName)) {
if ($localeLang == 'fr') {
$updateDataInSib['PRENOM'] = $firstName;
} else {
$updateDataInSib['NAME'] = $firstName;
}
}
if (!empty($lastName)) {
if ($localeLang == 'fr') {
$updateDataInSib['NOM'] = $lastName;
} else {
$updateDataInSib['SURNAME'] = $lastName;
}
}
if (!empty($firstName)) {
$updateDataInSib['CLIENT'] = 1;
} else {
$updateDataInSib['CLIENT'] = 0;
}
if (!empty($storeId)) {
$updateDataInSib['STORE_ID'] = $storeId;
}
if (!empty($storeView)) {
$updateDataInSib['MAGENTO_LANG'] = $storeView;
}
$this->sibModel->subscribeByruntime($email, $updateDataInSib);
$this->sibModel->sendWsTemplateMail($email);
}
}
}Altra cosa perfida erano le query dirette a database. Il “sibModel” precedentemente indicato tra le tante cose (1571 righe di codice, più avanti parleremo della divisione delle responsabilità della classe) effettuava anche delle query dirette a database per leggere dalla tabella dei customer e dei subscriber newsletter.
Non riporto l’intera classe ma un pezzo per vedere come andrebbe lavorato:
...
/**
* Description: check newsletter email status.
*
*/
public function checkNlStatus($email)
{
$connection=$this->createDbConnection();
$tblNewsletter = $this->tbWithPrefix('newsletter_subscriber');
$resultSubscriber = $connection->fetchAll('SELECT `subscriber_status` FROM `'.$tblNewsletter.'` WHERE subscriber_email ='."'$email'");
foreach ($resultSubscriber as $subsdata){
return $subscriberEmail = !empty($subsdata['subscriber_status']) ? $subsdata['subscriber_status'] : '';
}
}
...Questa pratica è particolarmente errata per due motivi principalmente:
– Magento ha già resource model per il singolo subscriber e per collection di subscribers e quindi stiamo scrivendo codice inutile e duplicato
– Non utilizzare i model del core ci espongono a problemi di manutenibilità: in un prossimo futuro Magento2 potrebbe utilizzare tabelle differenti per la gestione dei subscribers della newsletter, se utilizziamo le classi (rifacendoci alle interfacce) e i metodi di queste è molto più difficile che il mio codice non funzioni con una nuova versione.
Quindi questo pezzo di codice sarebbe dovuto essere qualcosa del genere:
...
public function __construct(
...
Magento\Newsletter\Model\Subscriber $subscriberModel
...
)
{
$this->subscriberModel = $subscriberModel;
...
}
...
/**
* Description: check newsletter email status.
*
*/
public function checkNlStatus($email)
{
$subscriber $this->subscriberModel->loadByEmail($email);
if ($subscriber->getId()) {
return $subscriber->getStatus();
}
return '';
}
...Esempi di questo tipo si ripetono in tutto il codice, puliti questi e fatto un po’ di refactoring si può procedere con l’organizzazione del codice e divisione delle responsabilità delle varie classi.
Divisione delle responsabilità e riorganizzazione del modulo(2):
Finita la pulizia, le classi erano comunque abnormi e rendevano difficile capire chi dovesse fare cosa. C’erano model, come nell’esempio precedente, che si occupavano di leggere configurazioni, leggere da database, inviare mail, reperire template da inviare per mail, tutto in un unico calderone. Questo è un grosso problema in quanto chi deve partecipare alla modifica e revisione del modulo sarà messo in difficoltà sia in una fase di lettura sia in una fase di scrittura di nuove feature. Dopo aver sistemato i vari model spezzando l’operatività in model dedicati, piuttosto che negli helper (per funzioni ripetute e “potenzialmente statiche”), … ho deciso di dividere il modulo in 3 estensioni separate.
– Modulo core dell’integrazione (sib-core) comprendente l’helper principale dedicato alla lettura delle configurazioni base, il model di accesso alle API sendinblue (non più curl dirette), richiesta come dipendenza delle SDK PHP appunto, logger centralizzato
– Modulo di gestione dei subscribers (sib-contact-sync) comprendente il setup di creazione delle tabelle custom necessarie a sendinblue su magento per la gestione dei contatti (ed i corrispondenti model, repositories e resource model), l’helper dedicato alla lettura delle configurazioni relative ai contatti, il model responsabile della sincronizzazione su sib dei contatti attraverso chiamate alle classi wrapper delle sdk presenti nel core, gestione degli eventi legati alle subscription
– Modulo di gestione degli ordini (sib-order-sync) comprendente la gestione degli eventi legati agli ordini e sincronizzazione dati d’ordine su sendinblue tramite sib-contact-sync (i dati d’ordine in sendinblue sono legati ai contatti stessi)
Conclusione:
Alla fine del lavoro sono riuscito a portare alla luce un modulo pulito con classi circoscritte e con responsabilità chiare, il modulo funziona per tutti i Magento2 dalla versione 2.3.x e 2.4.x (testato fino alla versione 2.4.5).
Ho proposto a sendinblue di adottare questo modulo, gratuitamente, citando il sottoscritto tra i contributori, tuttavia il loro customer service mi ha più volte rimbalzato ribadendo che stanno lavorando loro stessi all’aggiornamento del modulo! Con ultimo commit del 7 Dicembre 2020 ad oggi, 14 Dicembre 2022, ancora nulla. Speriamo in un 2023 migliore?
Il modulo è disponibile sul mio profilo github, lieto di ricevere contribuzioni, è utilizzato da alcuni miei clienti con successo e senza riscontrare bug di alcun genere.
Miglioramenti? Sincronizzare on the fly i dati su Sendinblue via API non è una grande cosa, io ho ereditato la logica dal vecchio modulo e per semplicità la ho portata avanti, tuttavia sarebbe molto meglio implementare un sistema di code utilizzando rabbitMQ. Qualunque altro spunto è gradito.