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

Bugfix: queued redirect #6655

Merged
merged 3 commits into from May 8, 2019

Conversation

Projects
6 participants
@jojomnky
Copy link

commented Sep 28, 2018

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

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

Description:

Adds an identifier isRedirect when pushing to the queue for Redirect vs Page
Consumer expects identifier isRedirect and looks up the correct entity accordingly.

Steps to reproduce the bug:

  1. Enable queued page_hit processing
  2. Create an email with a link to an off-site url
  3. Send the email and click the link in the received email.
  4. Run the queue processing command to consume the queued page_hit
  5. The click counter for the email will not increase

Steps to test this PR:

  1. Enable queued page_hit processing
  2. Create an email with a link to an off-site url
  3. Send the email and click the link in the received email.
  4. Run the queue processing command to consume the queued page_hit
  5. See the click counter for the email increase

List deprecations along with the new alternative:

none

List backwards compatibility breaks:

  1. Existing page_hit entries in the queue may not be processable with the new consumer

Tim Boesenberg added some commits Sep 28, 2018

Tim Boesenberg
adds 'isRedirect' to the `page_hit` queueing payload
consumer decides which type of entity to build (either Page or Redirect) based on 'isRedirect'
Tim Boesenberg

@npracht npracht added this to the 2.15.0 milestone Oct 2, 2018

@npracht npracht added this to To do in Testing 2.15.0 Oct 16, 2018

@Woeler Woeler removed this from To do in Testing 2.15.0 Dec 3, 2018

@Woeler Woeler modified the milestones: 2.15.0, 2.15.1 Dec 3, 2018

@heathdutton heathdutton added this to Code Review (2 required) in Mautic 2 Dec 6, 2018

@jojomnky jojomnky changed the title Bugfix/queued redirect Bugfix: queued redirect Jan 3, 2019

@alanhartless alanhartless added this to Needs Testing in 2.15.1 Jan 14, 2019

Mautic 2 automation moved this from Code Review (2 required) to Changes Requested / Review Jan 30, 2019

@@ -188,13 +188,20 @@ public function onPageHit(QueueConsumerEvent $event)
$trackingNewlyGenerated = $payload['isNew'];
$pageId = $payload['pageId'];
$leadId = $payload['leadId'];
$isRedirect = $payload['isRedirect'];

This comment has been minimized.

Copy link
@alanhartless

alanhartless Jan 30, 2019

Contributor

Let's put a !empty() around this to prevent any issues for jobs that were injected right before an update where isRedirect will not be defined.

@alanhartless alanhartless moved this from Needs Testing to Discussion in 2.15.1 Jan 30, 2019

Tim Boesenberg

Mautic 2 automation moved this from Changes Requested / Review to Ready to Test (first time) Jan 31, 2019

@npracht npracht moved this from Discussion to Needs Second Test in 2.15.1 Feb 6, 2019

@npracht npracht removed the Ready To Test label Feb 6, 2019

@alanhartless alanhartless removed this from Needs Second Test in 2.15.1 Mar 11, 2019

@alanhartless alanhartless modified the milestones: 2.15.1, 2.16.0 Mar 11, 2019

@npracht npracht modified the milestones: 2.16.0, 2.15.2 Mar 28, 2019

@npracht npracht moved this from Ready to Test (first time) to Ready to Test (confirmation) in Mautic 2 Apr 4, 2019

Mautic 2 automation moved this from Ready to Test (confirmation) to Ready to Test (first time) May 7, 2019

@escopecz escopecz moved this from Ready to Test (first time) to Ready to Commit (passed testing) in Mautic 2 May 7, 2019

@kuzmany kuzmany merged commit 1b1dbcc into mautic:staging May 8, 2019

2 checks passed

Scrutinizer Analysis: 3288 Issues, 576 Patches – Tests: passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

Mautic 2 automation moved this from Ready to Commit (passed testing) to Merged May 8, 2019

maxlawton added a commit to MauldinEconomics/mautic that referenced this pull request May 10, 2019

Fix merge conflict with PR mautic#6655
Merge branch 'bugfix/queued-redirect' into bugfix/reject-non-hits

* bugfix/queued-redirect:
  per @alanhartless request
  simplify conditional
  adds 'isRedirect' to the `page_hit` queueing payload consumer decides which type of entity to build (either Page or Redirect) based on 'isRedirect'
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.