[SPE-105] feat(analytics): add Segment PHP client bootstrap#23
[SPE-105] feat(analytics): add Segment PHP client bootstrap#23dylanDenizonPresta wants to merge 9 commits intomainfrom
Conversation
- Add segmentio/analytics-php dependency and PHP bootstrap (Segment::init) - Bootstrap from BO configuration context when module is enabled - Add Segment documentation and adjust tests accordingly
|
|
||
| Segment::init($writeKey); | ||
| self::$clientInitialized = true; | ||
| } |
There was a problem hiding this comment.
On pourrait ajouter une méthode générique pour initialiser & tracker les événements
Tu n'aurais plu besoin de la logique dans hookActionAdminControllerSetMedia
L’initialisation de Segment ne devrait pas dépendre d'un hook.
Exemple:
private const EVENT_GUEST_INIT_SUCCEEDED = 'opc_guest_init_succeeded';
public static function bootstrap(): void
{
if (self::$clientInitialized === true) {
return;
}
$writeKey = trim((string) self::SEGMENT_CLIENT_KEY_PHP);
if ($writeKey === '') {
return;
}
Segment::init($writeKey);
self::$clientInitialized = true;
}
/**
* @param array<string, mixed> $properties
* @param array<string, mixed> $context
*/
private static function track(
string $event,
array $properties = [],
array $context = []
): void {
try {
self::bootstrap();
if (self::$clientInitialized === false) {
return;
}
Segment::track([
'event' => $event,
'properties' => $properties,
'context' => $context,
]);
} catch (Throwable $exception) {
PrestaShopLogger::addLog(
sprintf('ps_onepagecheckout analytics track failed for "%s": %s', $event, $exception->getMessage()),
2
);
}
}
public static function trackGuestInitSucceeded(array $properties = []): void
{
self::track(EVENT_GUEST_INIT_SUCCEEDED, $properties);
}There was a problem hiding this comment.
@dylanDenizonPresta ce commentaire est bloquant et doit être résolu afin d'avoir un tracking robuste, réutilisable et centralisé
dylanDenizonPresta
left a comment
There was a problem hiding this comment.
Merci ! Pour éviter d'envoyer des events vers la prod en local/préprod, je propose de ne pas embarquer de write key en dur: on la lit depuis une variable d'environnement (ou un secret CI/serveur), avec une clé différente par environnement. Dans le module, on bootstrape Segment uniquement si la clé est non vide. Je peux pousser un fix qui remplace la constante par une lecture env (ex: PS_OPC_SEGMENT_WRITE_KEY) + doc.
Move Segment PHP write key out of the repository by reading it from PS_OPC_SEGMENT_WRITE_KEY. This prevents local/preprod environments from sending events to the production Segment source when the prod key is not present.
|
J’ai poussé un fix: la write key n’est plus en dur, elle est lue depuis la variable d’environnement (doc + tests mis à jour). Du coup chaque environnement (local/préprod/prod) peut avoir sa clé propre, et sans clé on n’initialise pas Segment. |
|
Correctif: la write key est lue depuis la variable d’environnement PS_OPC_SEGMENT_WRITE_KEY (plus de clé en dur). Doc + tests mis à jour. Sans clé, Segment n’est pas initialisé. |
Read Segment write keys from environment variables and select the correct one using APP_ENV. - APP_ENV=prod|production -> SEGMENT_PROD_KEY - otherwise -> SEGMENT_PREPROD_KEY
Use _PS_MODE_DEV_ to select which Segment write key to load from env. - _PS_MODE_DEV_=true -> SEGMENT_PREPROD_KEY - _PS_MODE_DEV_=false -> SEGMENT_PROD_KEY Also stop tracking .env and provide .env.dist template.
Summary
segmentio/analytics-phpand bootstrapSegment::init()fromAnalytics::bootstrap()when the module is enabled.actionAdminControllerSetMediaon module configuration BO context only.docs/SEGMENT.mdand link from README.Notes
Analytics::SEGMENT_CLIENT_KEY_PHP(empty by default until set).track/flush) is planned for follow-up work.Test plan
composer installat module root./scripts/run-tests.sh unit