diff --git a/lib/Cron/Workers/SendingQueue/SendingQueue.php b/lib/Cron/Workers/SendingQueue/SendingQueue.php index 3a6aad8bb0..5bf6af5d25 100644 --- a/lib/Cron/Workers/SendingQueue/SendingQueue.php +++ b/lib/Cron/Workers/SendingQueue/SendingQueue.php @@ -5,6 +5,7 @@ use MailPoet\Cron\CronHelper; use MailPoet\Cron\Workers\SendingQueue\Tasks\Links; use MailPoet\Cron\Workers\SendingQueue\Tasks\Mailer as MailerTask; use MailPoet\Cron\Workers\SendingQueue\Tasks\Newsletter as NewsletterTask; +use MailPoet\Logger; use MailPoet\Mailer\MailerLog; use MailPoet\Models\Newsletter as NewsletterModel; use MailPoet\Models\SendingQueue as SendingQueueModel; @@ -190,4 +191,4 @@ class SendingQueue { ->whereNull('type') ->findMany(); } -} \ No newline at end of file +} diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Links.php b/lib/Cron/Workers/SendingQueue/Tasks/Links.php index 8ff2dc621d..f01780a423 100644 --- a/lib/Cron/Workers/SendingQueue/Tasks/Links.php +++ b/lib/Cron/Workers/SendingQueue/Tasks/Links.php @@ -15,15 +15,15 @@ if(!defined('ABSPATH')) exit; class Links { static function process($rendered_newsletter, $newsletter, $queue) { list($rendered_newsletter, $links) = - self::hashAndReplaceLinks($rendered_newsletter); + self::hashAndReplaceLinks($rendered_newsletter, $newsletter->id, $queue->id); self::saveLinks($links, $newsletter, $queue); return $rendered_newsletter; } - static function hashAndReplaceLinks($rendered_newsletter) { + static function hashAndReplaceLinks($rendered_newsletter, $newsletter_id, $queue_id) { // join HTML and TEXT rendered body into a text string $content = Helpers::joinObject($rendered_newsletter); - list($content, $links) = NewsletterLinks::process($content); + list($content, $links) = NewsletterLinks::process($content, $newsletter_id, $queue_id); // split the processed body with hashed links back to HTML and TEXT list($rendered_newsletter['html'], $rendered_newsletter['text']) = Helpers::splitObject($content); diff --git a/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php b/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php index 5c7623a5de..90e39494ca 100644 --- a/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php +++ b/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php @@ -5,6 +5,7 @@ namespace MailPoet\Cron\Workers\SendingQueue\Tasks; use MailPoet\Cron\Workers\SendingQueue\Tasks\Links as LinksTask; use MailPoet\Cron\Workers\SendingQueue\Tasks\Posts as PostsTask; use MailPoet\Cron\Workers\SendingQueue\Tasks\Shortcodes as ShortcodesTask; +use MailPoet\Logger; use MailPoet\Mailer\MailerLog; use MailPoet\Models\Newsletter as NewsletterModel; use MailPoet\Models\NewsletterSegment as NewsletterSegmentModel; @@ -175,4 +176,4 @@ class Newsletter { __('There was an error processing your newsletter during sending. If possible, please contact us and report this issue.') ); } -} \ No newline at end of file +} diff --git a/lib/Newsletter/Links/Links.php b/lib/Newsletter/Links/Links.php index 990115dab1..d74c7a5804 100644 --- a/lib/Newsletter/Links/Links.php +++ b/lib/Newsletter/Links/Links.php @@ -1,6 +1,7 @@ whereEqual('queue_id', $queue_id) + ->findMany(); + $saved_links = array(); + foreach ($links as $link) { + $saved_links[$link->url] = $link->asArray(); + } + return $saved_links; + } + + static function hash($extracted_links, $saved_links) { + $processed_links = array_map(function(&$link) { + $link['type'] = Links::LINK_TYPE_URL; + $link['link'] = $link['url']; + $link['processed_link'] = self::DATA_TAG_CLICK . '-' . $link['hash']; + return $link; + }, $saved_links); foreach($extracted_links as $extracted_link) { + $link = $extracted_link['link']; + if (array_key_exists($link, $processed_links)) + continue; $hash = Security::generateHash(); // Use URL as a key to map between extracted and processed links // regardless of their sequential position (useful for link skips etc.) - $link = $extracted_link['link']; $processed_links[$link] = array( 'type' => $extracted_link['type'], 'hash' => $hash, @@ -137,6 +160,8 @@ class Links { static function save(array $links, $newsletter_id, $queue_id) { foreach($links as $link) { + if (isset($link['id'])) + continue; if(empty($link['hash']) || empty($link['link'])) continue; $newsletter_link = NewsletterLink::create(); $newsletter_link->newsletter_id = $newsletter_id; @@ -198,4 +223,4 @@ class Links { $transformed_data['preview'] = (!empty($data[4])) ? $data[4] : false; return $transformed_data; } -} \ No newline at end of file +} diff --git a/tests/unit/Cron/Workers/SendingQueue/Tasks/LinksTest.php b/tests/unit/Cron/Workers/SendingQueue/Tasks/LinksTest.php index 53b67dce1a..cdd05e235a 100644 --- a/tests/unit/Cron/Workers/SendingQueue/Tasks/LinksTest.php +++ b/tests/unit/Cron/Workers/SendingQueue/Tasks/LinksTest.php @@ -30,7 +30,7 @@ class LinksTest extends \MailPoetTest { 'html' => 'Example Link', 'text' => 'Example Link' ); - $result = Links::hashAndReplaceLinks($rendered_newsletter); + $result = Links::hashAndReplaceLinks($rendered_newsletter, 0, 0); $processed_rendered_newsletter_body = $result[0]; $processed_and_hashed_links = $result[1]; expect($processed_rendered_newsletter_body['html']) @@ -59,4 +59,4 @@ class LinksTest extends \MailPoetTest { \ORM::raw_execute('TRUNCATE ' . Newsletter::$_table); \ORM::raw_execute('TRUNCATE ' . NewsletterLink::$_table); } -} \ No newline at end of file +} diff --git a/tests/unit/Newsletter/Links/LinksTest.php b/tests/unit/Newsletter/Links/LinksTest.php index 01389a98f3..babad27fad 100644 --- a/tests/unit/Newsletter/Links/LinksTest.php +++ b/tests/unit/Newsletter/Links/LinksTest.php @@ -24,7 +24,7 @@ class LinksTest extends \MailPoetTest { function testItOnlyHashesAndReplacesLinksInAnchorTags() { $template = ''; - $result = Links::process($template); + $result = Links::process($template, 0, 0); expect($result[0])->equals( sprintf( '', @@ -34,6 +34,25 @@ class LinksTest extends \MailPoetTest { ); } + function testItDoesnotRehashExistingLinks() { + $link = NewsletterLink::create(); + $link->newsletter_id = 3; + $link->queue_id = 3; + $link->hash = 123; + $link->url = 'http://example.com'; + $link->save(); + + $template = ''; + $result = Links::process($template, 3, 3); + expect($result[0])->equals( + sprintf( + '', + Links::DATA_TAG_CLICK, + 123 + ) + ); + } + function testItCanExtactLinkShortcodes() { $template = '[notlink:shortcode] [link:some_link_shortcode]'; $result = Links::extract($template); @@ -48,7 +67,7 @@ class LinksTest extends \MailPoetTest { function testItHashesAndReplacesLinks() { $template = 'some site [link:some_link_shortcode]'; - list($updated_content, $hashed_links) = Links::process($template); + list($updated_content, $hashed_links) = Links::process($template, 0, 0); // 2 links were hashed expect(count($hashed_links))->equals(2); @@ -63,7 +82,7 @@ class LinksTest extends \MailPoetTest { function testItHashesAndReplacesLinksWithSpecialCharacters() { $template = 'some site'; - $result = Links::process($template); + $result = Links::process($template, 0, 0); expect($result[0])->equals( sprintf( 'some site', @@ -171,6 +190,28 @@ class LinksTest extends \MailPoetTest { expect($newsltter_link->url)->equals('http://example.com'); } + function testItCanLoadLinks() { + $link = NewsletterLink::create(); + $link->newsletter_id = 1; + $link->queue_id = 2; + $link->hash = 123; + $link->url = 'http://example.com'; + $link->save(); + + $link = NewsletterLink::create(); + $link->newsletter_id = 1; + $link->queue_id = 3; + $link->hash = 456; + $link->url = 'http://demo.com'; + $link->save(); + + $links = Links::load(1, 2); + expect(is_array($links))->true(); + expect(count($links))->equals(1); + expect($links['http://example.com']['hash'])->equals(123); + expect($links['http://example.com']['url'])->equals('http://example.com'); + } + function testItMatchesHashedLinks() { $regex = Links::getLinkRegex(); expect((boolean)preg_match($regex, '[some_tag]-123'))->false();