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

Focus View dispatch event #6459

Merged
merged 3 commits into from Oct 15, 2018

Conversation

Projects
3 participants
@kuzmany
Copy link
Contributor

kuzmany commented Aug 13, 2018

Please be sure you are submitting this against the staging branch.

Q A
Bug fix?
New feature?
Automated tests included?
Related user documentation PR URL
Related developer documentation PR URL
Issues addressed (#s or URLs)
BC breaks?
Deprecations?

Description:

This PR add dispatch event If Focus pixel is viewed. Useful for developers they can store/trigger or something after focus pixel is viewed.

Steps to test this PR:

  1. Add tracking code and focus to page
  2. Install
    MauticFocusViewTestBundle.zip
    (copy to plugins, clear cache and go to Plugins and click to Install/Upgrade button)
  3. Then reload page with tracking code and focus.
  4. Focus should be viewed
    5: Then go to logs and see debug line

Focus #1 viewed. Works

image

Should works properly

List deprecations along with the new alternative:

List backwards compatibility breaks:

@kuzmany kuzmany added Ready To Test and removed WIP labels Sep 21, 2018

@kuzmany kuzmany added this to the 2.14.2 milestone Sep 21, 2018

@escopecz
Copy link
Member

escopecz left a comment

I have a couple of tiny suggestions

<?php
/*
* @copyright 2017 Mautic Contributors. All rights reserved

This comment has been minimized.

@escopecz

escopecz Sep 24, 2018

Member

Update the year please

public function __construct(Stat $stat)
{
$this->stat = $stat;
$this->focus = $stat->getFocus();

This comment has been minimized.

@escopecz

escopecz Sep 24, 2018

Member

This seems redundant to me. The listeners can do this as well. You could extract all props from the stat entity like Lead. Let's remove this and keep the event simple.

This comment has been minimized.

@kuzmany

kuzmany Sep 25, 2018

Author Contributor

I would like use Stat $stat in event.
In my plugin I need work with stat id and date_added to stat.
Than what you recommend? Just remove focus property? Or add another props to event , somesthin like

public function __construct(Stat $stat, Focus $focus, Lead $contact)

?

This comment has been minimized.

@escopecz

escopecz Sep 25, 2018

Member

I'd leave only the $stat property in the FocusViewEvent class. In your listener you can

$stat = $event->getStat();
$focus = $stat->getFocus();
$contact = $stat->getLead();
// ... and so on

Setting the $focus property to the event from the $stat entity seems like redundant code to me as you can do the same thing in the listener.

This comment has been minimized.

@kuzmany

kuzmany Sep 25, 2018

Author Contributor

Now I got it.
Thanks

@escopecz escopecz added this to To Do in Testing 2.14.2 Oct 2, 2018

@escopecz
Copy link
Member

escopecz left a comment

Looks good to me 👍

@escopecz escopecz moved this from To Do to Tested Once in Testing 2.14.2 Oct 5, 2018

@Enc3phale
Copy link
Contributor

Enc3phale left a comment

Work as expected, thanks!

[2018-10-15 10:21:19] mautic.DEBUG: CONTACT: New lead created with ID# 68.
[2018-10-15 10:21:19] mautic.DEBUG: CONTACT: Tracking session for contact ID# 68 through GET /index_dev.php/focus/3/viewpixel.gif  
[2018-10-15 10:21:19] mautic.DEBUG: LEAD: Tracking ID for this device is bgmdqnxwmo9839rwznzo7d3  
[2018-10-15 10:21:20] mautic.DEBUG: Focus #3 viewed. Works

@escopecz escopecz merged commit de2111a into mautic:staging Oct 15, 2018

2 checks passed

Scrutinizer Analysis: 4 updated code elements – Tests: passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

Testing 2.14.2 automation moved this from Tested Once to Merged Oct 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.