Fixed Twitter username link bugs

* Doesn't link isolated @ symbols * Fixed links to usernames surrounded by parentheses * Fixed links to usernames starting with a . * Simplified the replacement of usernames using a regex * Added test cases for each to assert links are correct * Closes ThinkUpLLC#373, Closes ThinkUpLLC#374, Closes ThinkUpLLC#375, Closes ThinkUpLLC#381, Closes ThinkUpLLC#543
unruthless · Jan 9, 2011 · 33f9831 · 33f9831
1 parent 9c83a6b
commit 33f9831
Show file tree

Hide file tree

Showing 4 changed files with 107 additions and 41 deletions.
diff --git a/tests/TestOfSmartyModifierLinkUsernames.php b/tests/TestOfSmartyModifierLinkUsernames.php
@@ -0,0 +1,89 @@
+<?php
+/**
+ *
+ * ThinkUp/tests/TestOfSmartyModifierLinkUsernames.php
+ *
+ * Copyright (c) 2009-2010 Gina Trapani
+ *
+ * LICENSE:
+ *
+ * This file is part of ThinkUp (http://thinkupapp.com).
+ *
+ * ThinkUp is free software: you can redistribute it and/or modify it under the terms of the GNU General Public
+ * License as published by the Free Software Foundation, either version 2 of the License, or (at your option) any
+ * later version.
+ *
+ * ThinkUp is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied
+ * warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License along with ThinkUp.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * @author Gina Trapani <ginatrapani[at]gmail[dot]com>
+ * @license http://www.gnu.org/licenses/gpl.html
+ * @copyright 2009-2010
+ */
+
+require_once dirname(__FILE__).'/init.tests.php';
+require_once THINKUP_ROOT_PATH.'webapp/_lib/extlib/simpletest/autorun.php';
+require_once THINKUP_ROOT_PATH.'webapp/_lib/view/plugins/modifier.link_usernames.php';
+require_once THINKUP_ROOT_PATH.'webapp/_lib/view/plugins/modifier.link_usernames_to_twitter.php';
+
+class TestOfSmartyModiferLinkUsernames extends ThinkUpBasicUnitTestCase {
+    /**
+     *
+     * @var array string
+     */
+    var $test_tweets;
+    /**
+     *
+     * @var array string
+     */
+    var $internally_linked_tweets;
+    /**
+     *
+     * @var array string
+     */
+    var $externally_linked_tweets;
+
+    public function __construct() {
+        $this->UnitTestCase('Link Twitter usernames Smarty modifier test');
+        $this->test_tweets = array(
+        "Hey @anildash think this up!", 
+        "If you're interested, @ me details", 
+        ".@anildash thinks so",
+        "This is a tweet with multiple usernames like @waxpancake and @thinkupapp",
+        "Blah blah blah (@username). Blah blah");
+
+        $this->internally_linked_tweets = array(
+        'Hey <a href="/user/?u=anildash&n=twitter&i=me">@anildash</a> think this up!', 
+        "If you're interested, @ me details", 
+        '.<a href="/user/?u=anildash&n=twitter&i=me">@anildash</a> thinks so',
+        'This is a tweet with multiple usernames like <a href="/user/?u=waxpancake&n=twitter&i=me">@waxpancake</a> '.
+        'and <a href="/user/?u=thinkupapp&n=twitter&i=me">@thinkupapp</a>',
+        'Blah blah blah (<a href="/user/?u=username&n=twitter&i=me">@username</a>). Blah blah');
+
+        $this->externally_linked_tweets = array(
+        'Hey <a href="http://twitter.com/anildash">@anildash</a> think this up!', 
+        "If you're interested, @ me details", 
+        '.<a href="http://twitter.com/anildash">@anildash</a> thinks so',
+        'This is a tweet with multiple usernames like <a href="http://twitter.com/waxpancake">@waxpancake</a> '.
+        'and <a href="http://twitter.com/thinkupapp">@thinkupapp</a>',
+        'Blah blah blah (<a href="http://twitter.com/username">@username</a>). Blah blah');
+    }
+
+    public function testLinks() {
+        //test internal links
+        foreach ($this->test_tweets as $index => $test_tweet) {
+            $linked_tweet = smarty_modifier_link_usernames($test_tweet, "me", "twitter");
+            $this->assertEqual($this->internally_linked_tweets[$index], $linked_tweet);
+        }
+
+        //test Twitter.com links
+        foreach ($this->test_tweets as $index => $test_tweet) {
+            $linked_tweet = smarty_modifier_link_usernames_to_twitter($test_tweet, "me", "twitter");
+            $this->assertEqual($this->externally_linked_tweets[$index], $linked_tweet);
+        }
+    }
+}
diff --git a/tests/all_plugin_tests.php b/tests/all_plugin_tests.php
@@ -59,6 +59,7 @@
 require_once THINKUP_ROOT_PATH.'webapp/plugins/geoencoder/tests/TestOfGeoEncoderPluginConfigurationController.php';
 require_once THINKUP_ROOT_PATH.'webapp/plugins/geoencoder/tests/TestOfMapController.php';
 require_once THINKUP_ROOT_PATH.'webapp/plugins/hellothinkup/tests/TestOfHelloThinkUpPluginConfigurationController.php';
+require_once THINKUP_ROOT_PATH.'tests/TestOfSmartyModifierLinkUsernames.php';
 
 $plugin_tests = & new GroupTest('Plugin tests');
 $plugin_tests->addTestCase(new TestOfExpandURLsPlugin());
@@ -80,5 +81,6 @@
 $plugin_tests->addTestCase(new TestOfURLProcessor());
 $plugin_tests->addTestCase(new TestOfRetweetDetector());
 $plugin_tests->addTestCase(new TestOfHelloThinkUpPluginConfigurationController());
+$plugin_tests->addTestCase(new TestOfSmartyModiferLinkUsernames());
 
 $plugin_tests->run( new TextReporter());
diff --git a/webapp/_lib/view/plugins/modifier.link_usernames.php b/webapp/_lib/view/plugins/modifier.link_usernames.php
@@ -9,17 +9,17 @@
  *
  * This file is part of ThinkUp (http://thinkupapp.com).
  *
- * ThinkUp is free software: you can redistribute it and/or modify it under the terms of the GNU General Public 
- * License as published by the Free Software Foundation, either version 2 of the License, or (at your option) any 
+ * ThinkUp is free software: you can redistribute it and/or modify it under the terms of the GNU General Public
+ * License as published by the Free Software Foundation, either version 2 of the License, or (at your option) any
  * later version.
  *
- * ThinkUp is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied 
- * warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more 
+ * ThinkUp is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied
+ * warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
  * details.
  *
- * You should have received a copy of the GNU General Public License along with ThinkUp.  If not, see 
+ * You should have received a copy of the GNU General Public License along with ThinkUp.  If not, see
  * <http://www.gnu.org/licenses/>.
-*/
+ */
 /**
  * Smarty plugin
  * @package Smarty
@@ -43,21 +43,8 @@
  * @return string
  */
 function smarty_modifier_link_usernames($text, $instance_username, $network) {
-    //TODO: Find a more elegant way to do this that's totally regex-based, not loving this explod/implode approach
     $config = Config::getInstance();
-    $words = explode(" ", $text);
-    $pattern = '/^@[a-zA-Z0-9_]+/';
-    for($k = 0; $k < count($words); $k++) {
-        if ( substr($words[$k], 0, 1) == '@' ) {
-            preg_match($pattern, $words[$k], $matches);
-            $words[$k] = '<a href="'.$config->getValue('site_root_path').'user/?u='.substr($matches[0],1).
-            '&n='.$network.'&i='.$instance_username.'">'.$words[$k].'</a>';
-        } else if ( substr($words[$k], 0, 2) == '(@' ) { //for usersnames in parentheses
-            preg_match($pattern, substr($words[$k], 1, strlen($words[$k])), $matches);
-            $words[$k] = '<a href="'.$config->getValue('site_root_path').'user/?u='.substr($matches[0],1).
-            '&n='.$network.'&i='.$instance_username.'">'.$words[$k].'</a>';
-        }
-    }
-    return implode($words, ' ');
+    $site_root_path = $config->getValue('site_root_path');
+    return preg_replace('/(^|[^a-z0-9_])@([a-z0-9_]+)/i', '$1<a href="'.$site_root_path.'user/?u=$2&n='.$network.
+    '&i='.$instance_username.'">@$2</a>', $text);
 }
-?>
diff --git a/webapp/_lib/view/plugins/modifier.link_usernames_to_twitter.php b/webapp/_lib/view/plugins/modifier.link_usernames_to_twitter.php
@@ -9,17 +9,17 @@
  *
  * This file is part of ThinkUp (http://thinkupapp.com).
  *
- * ThinkUp is free software: you can redistribute it and/or modify it under the terms of the GNU General Public 
- * License as published by the Free Software Foundation, either version 2 of the License, or (at your option) any 
+ * ThinkUp is free software: you can redistribute it and/or modify it under the terms of the GNU General Public
+ * License as published by the Free Software Foundation, either version 2 of the License, or (at your option) any
  * later version.
  *
- * ThinkUp is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied 
- * warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more 
+ * ThinkUp is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied
+ * warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
  * details.
  *
- * You should have received a copy of the GNU General Public License along with ThinkUp.  If not, see 
+ * You should have received a copy of the GNU General Public License along with ThinkUp.  If not, see
  * <http://www.gnu.org/licenses/>.
-*/
+ */
 /**
  * Smarty plugin
  * @package Smarty
@@ -44,17 +44,5 @@
  * @return string
  */
 function smarty_modifier_link_usernames_to_twitter($text) {
-    $words = explode(" ", $text);
-    $pattern = '/^@[a-zA-Z0-9_]+/';
-    for($k = 0; $k < count($words); $k++) {
-        if ( substr($words[$k], 0, 1) == '@' ) {
-            preg_match($pattern, $words[$k], $matches);
-            $words[$k] = '<a href="http://twitter.com/'.substr($matches[0],1).'">'.$words[$k].'</a>';
-        } else if ( substr($words[$k], 0, 2) == '(@' ) { //for usersnames in parentheses
-            preg_match($pattern, substr($words[$k], 1, strlen($words[$k])), $matches);
-            $words[$k] = '<a href="http://twitter.com/'.substr($matches[0],1).'">'.$words[$k].'</a>';
-        }
-    }
-    return implode($words, ' ');
+    return preg_replace('/(^|[^a-z0-9_])@([a-z0-9_]+)/i', '$1<a href="http://twitter.com/$2">@$2</a>', $text);
 }
-?>