Fixed wrong task link generation within Markdown text

lvic · Apr 29, 2016 · fc8f874 · fc8f874
1 parent 81a25cb
commit fc8f874
Show file tree

Hide file tree

Showing 8 changed files with 124 additions and 83 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -22,6 +22,7 @@ Improvements:
 
 Bug fixes:
 
+* Fixed wrong task link generation within Markdown text
 * Fixed wrong URL on comment toggle link for sorting
 * Fixed form submission with Meta+Enter keyboard shortcut
 * Removed PHP notices in comment suppression view

diff --git a/app/Core/Markdown.php b/app/Core/Markdown.php
@@ -15,12 +15,12 @@
 class Markdown extends Parsedown
 {
     /**
-     * Link params for tasks
+     * Task links generated will use the project token instead
      *
      * @access private
-     * @var array
+     * @var boolean
      */
-    private $link = array();
+    private $isPublicLink = false;
 
     /**
      * Container
@@ -35,11 +35,11 @@ class Markdown extends Parsedown
      *
      * @access public
      * @param  Container  $container
-     * @param  array      $link
+     * @param  boolean    $isPublicLink
      */
-    public function __construct(Container $container, array $link)
+    public function __construct(Container $container, $isPublicLink)
     {
-        $this->link = $link;
+        $this->isPublicLink = $isPublicLink;
         $this->container = $container;
         $this->InlineTypes['#'][] = 'TaskLink';
         $this->InlineTypes['@'][] = 'UserLink';
@@ -53,26 +53,26 @@ public function __construct(Container $container, array $link)
      *
      * @access public
      * @param  array  $Excerpt
-     * @return array
+     * @return array|null
      */
     protected function inlineTaskLink(array $Excerpt)
     {
-        if (! empty($this->link) && preg_match('!#(\d+)!i', $Excerpt['text'], $matches)) {
-            $url = $this->container['helper']->url->href(
-                $this->link['controller'],
-                $this->link['action'],
-                $this->link['params'] + array('task_id' => $matches[1])
-            );
-
-            return array(
-                'extent' => strlen($matches[0]),
-                'element' => array(
-                    'name' => 'a',
-                    'text' => $matches[0],
-                    'attributes' => array('href' => $url)
-                ),
-            );
+        if (preg_match('!#(\d+)!i', $Excerpt['text'], $matches)) {
+            $link = $this->buildTaskLink($matches[1]);
+
+            if (! empty($link)) {
+                return array(
+                    'extent' => strlen($matches[0]),
+                    'element' => array(
+                        'name' => 'a',
+                        'text' => $matches[0],
+                        'attributes' => array('href' => $link),
+                    ),
+                );
+            }
         }
+
+        return null;
     }
 
     /**
@@ -82,11 +82,11 @@ protected function inlineTaskLink(array $Excerpt)
      *
      * @access public
      * @param  array  $Excerpt
-     * @return array
+     * @return array|null
      */
     protected function inlineUserLink(array $Excerpt)
     {
-        if (preg_match('/^@([^\s]+)/', $Excerpt['text'], $matches)) {
+        if (! $this->isPublicLink && preg_match('/^@([^\s]+)/', $Excerpt['text'], $matches)) {
             $user_id = $this->container['user']->getIdByUsername($matches[1]);
 
             if (! empty($user_id)) {
@@ -102,5 +102,40 @@ protected function inlineUserLink(array $Excerpt)
                 );
             }
         }
+
+        return null;
+    }
+
+    /**
+     * Build task link
+     *
+     * @access private
+     * @param  integer $task_id
+     * @return string
+     */
+    private function buildTaskLink($task_id)
+    {
+        if ($this->isPublicLink) {
+            $token = $this->container['memoryCache']->proxy($this->container['taskFinder'], 'getProjectToken', $task_id);
+
+            if (! empty($token)) {
+                return $this->container['helper']->url->href(
+                    'task',
+                    'readonly',
+                    array(
+                        'token' => $token,
+                        'task_id' => $task_id,
+                    )
+                );
+            }
+
+            return '';
+        }
+
+        return $this->container['helper']->url->href(
+            'task',
+            'show',
+            array('task_id' => $task_id)
+        );
     }
 }
diff --git a/app/Helper/TextHelper.php b/app/Helper/TextHelper.php
@@ -27,13 +27,13 @@ public function e($value)
     /**
      * Markdown transformation
      *
-     * @param  string    $text     Markdown content
-     * @param  array     $link     Link parameters for replacement
+     * @param  string    $text
+     * @param  boolean   $isPublicLink
      * @return string
      */
-    public function markdown($text, array $link = array())
+    public function markdown($text, $isPublicLink = false)
     {
-        $parser = new Markdown($this->container, $link);
+        $parser = new Markdown($this->container, $isPublicLink);
         $parser->setMarkupEscaped(MARKDOWN_ESCAPE_HTML);
         return $parser->text($text);
     }

diff --git a/app/Model/TaskFinder.php b/app/Model/TaskFinder.php
@@ -452,4 +452,20 @@ public function exists($task_id)
     {
         return $this->db->table(Task::TABLE)->eq('id', $task_id)->exists();
     }
+
+    /**
+     * Get project token
+     *
+     * @access public
+     * @param  integer $task_id
+     * @return string
+     */
+    public function getProjectToken($task_id)
+    {
+        return $this->db
+            ->table(Task::TABLE)
+            ->eq(Task::TABLE.'.id', $task_id)
+            ->join(Project::TABLE, 'id', 'project_id')
+            ->findOneColumn(Project::TABLE.'.token');
+    }
 }
diff --git a/app/Template/comment/show.php b/app/Template/comment/show.php
@@ -12,29 +12,7 @@
 
     <div class="comment-content">
         <div class="markdown">
-            <?php if (isset($is_public) && $is_public): ?>
-                <?= $this->text->markdown(
-                    $comment['comment'],
-                    array(
-                        'controller' => 'task',
-                        'action' => 'readonly',
-                        'params' => array(
-                            'token' => $project['token']
-                        )
-                    )
-                ) ?>
-            <?php else: ?>
-                <?= $this->text->markdown(
-                    $comment['comment'],
-                    array(
-                        'controller' => 'task',
-                        'action' => 'show',
-                        'params' => array(
-                            'project_id' => $task['project_id']
-                        )
-                    )
-                ) ?>
-            <?php endif ?>
+            <?= $this->text->markdown($comment['comment'], isset($is_public) && $is_public) ?>
         </div>
     </div>
 

diff --git a/app/Template/task/description.php b/app/Template/task/description.php
@@ -4,29 +4,7 @@
     </div>
     <div class="accordion-content">
         <article class="markdown">
-            <?php if (! isset($is_public)): ?>
-                <?= $this->text->markdown(
-                    $task['description'],
-                    array(
-                        'controller' => 'task',
-                        'action' => 'show',
-                        'params' => array(
-                            'project_id' => $task['project_id']
-                        )
-                    )
-                ) ?>
-            <?php else: ?>
-                <?= $this->text->markdown(
-                    $task['description'],
-                    array(
-                        'controller' => 'task',
-                        'action' => 'readonly',
-                        'params' => array(
-                            'token' => $project['token']
-                        )
-                    )
-                ) ?>
-            <?php endif ?>
+            <?= $this->text->markdown($task['description'], isset($is_public) && $is_public) ?>
         </article>
     </div>
-</section>
+</section>
diff --git a/tests/units/Helper/TextHelperTest.php b/tests/units/Helper/TextHelperTest.php
@@ -3,30 +3,43 @@
 require_once __DIR__.'/../Base.php';
 
 use Kanboard\Helper\TextHelper;
+use Kanboard\Model\Project;
+use Kanboard\Model\TaskCreation;
 
 class TextHelperTest extends Base
 {
     public function testMarkdownTaskLink()
     {
-        $h = new TextHelper($this->container);
+        $helper = new TextHelper($this->container);
+        $projectModel = new Project($this->container);
+        $taskCreationModel = new TaskCreation($this->container);
+
+        $this->assertEquals(1, $projectModel->create(array('name' => 'Project #1')));
+        $this->assertTrue($projectModel->enablePublicAccess(1));
+        $this->assertEquals(1, $taskCreationModel->create(array('title' => 'Task #1', 'project_id' => 1)));
+        $project = $projectModel->getById(1);
+
+        $this->assertEquals('<p>Test</p>', $helper->markdown('Test'));
 
-        $this->assertEquals('<p>Test</p>', $h->markdown('Test'));
+        $this->assertEquals(
+            '<p>Task <a href="?controller=task&amp;action=show&amp;task_id=123">#123</a></p>',
+            $helper->markdown('Task #123')
+        );
 
         $this->assertEquals(
             '<p>Task #123</p>',
-            $h->markdown('Task #123')
+            $helper->markdown('Task #123', true)
         );
 
         $this->assertEquals(
-            '<p>Task <a href="?controller=a&amp;action=b&amp;c=d&amp;task_id=123">#123</a></p>',
-            $h->markdown('Task #123', array('controller' => 'a', 'action' => 'b', 'params' => array('c' => 'd')))
+            '<p>Task <a href="?controller=task&amp;action=readonly&amp;token='.$project['token'].'&amp;task_id=1">#1</a></p>',
+            $helper->markdown('Task #1', true)
         );
 
         $this->assertEquals(
             '<p>Check that: <a href="http://stackoverflow.com/questions/1732348/regex-match-open-tags-except-xhtml-self-contained-tags/1732454#1732454">http://stackoverflow.com/questions/1732348/regex-match-open-tags-except-xhtml-self-contained-tags/1732454#1732454</a></p>',
-            $h->markdown(
-                'Check that: http://stackoverflow.com/questions/1732348/regex-match-open-tags-except-xhtml-self-contained-tags/1732454#1732454',
-                array('controller' => 'a', 'action' => 'b', 'params' => array('c' => 'd'))
+            $helper->markdown(
+                'Check that: http://stackoverflow.com/questions/1732348/regex-match-open-tags-except-xhtml-self-contained-tags/1732454#1732454'
             )
         );
     }
@@ -35,6 +48,7 @@ public function testMarkdownUserLink()
     {
         $h = new TextHelper($this->container);
         $this->assertEquals('<p>Text <a href="?controller=user&amp;action=profile&amp;user_id=1" class="user-mention-link">@admin</a> @notfound</p>', $h->markdown('Text @admin @notfound'));
+        $this->assertEquals('<p>Text @admin @notfound</p>', $h->markdown('Text @admin @notfound', true));
     }
 
     public function testFormatBytes()

diff --git a/tests/units/Model/TaskFinderTest.php b/tests/units/Model/TaskFinderTest.php
@@ -93,4 +93,23 @@ public function testCountByProject()
         $this->assertEquals(1, $tf->countByProjectId(1));
         $this->assertEquals(2, $tf->countByProjectId(2));
     }
+
+    public function testGetProjectToken()
+    {
+        $taskCreationModel = new TaskCreation($this->container);
+        $taskFinderModel = new TaskFinder($this->container);
+        $projectModel = new Project($this->container);
+
+        $this->assertEquals(1, $projectModel->create(array('name' => 'Project #1')));
+        $this->assertEquals(2, $projectModel->create(array('name' => 'Project #2')));
+
+        $this->assertTrue($projectModel->enablePublicAccess(1));
+
+        $this->assertEquals(1, $taskCreationModel->create(array('title' => 'Task #1', 'project_id' => 1)));
+        $this->assertEquals(2, $taskCreationModel->create(array('title' => 'Task #2', 'project_id' => 2)));
+
+        $project = $projectModel->getById(1);
+        $this->assertEquals($project['token'], $taskFinderModel->getProjectToken(1));
+        $this->assertEmpty($taskFinderModel->getProjectToken(2));
+    }
 }