1. This site uses cookies. By continuing to use this site, you are agreeing to our use of cookies. Learn More.

How to get the thread title from Xenforo_ControllerPublic_Thread

Discussion in 'XenForo Development Discussions' started by AndyB, Oct 16, 2013.

  1. AndyB

    AndyB Well-Known Member

    I have extended the Xenforo_ControllerPublic_Thread in my add-on. I would like to get the thread title. I've tried the following code with no luck:

    $threadTitle = $this->_input->filterSingle('title', XenForo_Input::STRING);
    $threadTitle = $title;
    $threadTitle = $thread['title'];

    The following code works to get the threadId:

    $threadId = $this->_input->filterSingle('thread_id', XenForo_Input::UINT);

    I suppose I could use a query to get the title based on the $threadId, but I'm hoping there's some code to get the thread title directly.

    Thank you.
     
  2. xf_phantom

    xf_phantom Well-Known Member

    Marcus, Chris D and AndyB like this.
  3. Chris D

    Chris D XenForo Developer Staff Member

    If you're working with a thread, it's more than reasonable to perform a query to get the entire thread record, and maybe even the forum record too.

    99% of the time, in the XenForo core code, they fetch the thread like this:

    PHP:
            $threadId $this->_input->filterSingle('thread_id'XenForo_Input::UINT);

            
    $ftpHelper $this->getHelper('ForumThreadPost');
            list(
    $threadFetchOptions$forumFetchOptions) = $this->_getThreadForumFetchOptions();
            list(
    $thread$forum) = $ftpHelper->assertThreadValidAndViewable($threadId$threadFetchOptions$forumFetchOptions);
    That makes sure the user can actually view the thread and forum.

    EDIT: Yeah exactly like xf_phantom suggested in the previous thread :)
     
    AndyB likes this.
  4. AndyB

    AndyB Well-Known Member

    Thank you, xf_phantom and Chris. I sure appreciate the help from both of you.

    This code works perfectly.

    PHP:
            // get thread title    
            
    $threadId $this->_input->filterSingle('thread_id'XenForo_Input::UINT);
            
    $ftpHelper $this->getHelper('ForumThreadPost');
            list(
    $threadFetchOptions$forumFetchOptions) = $this->_getThreadForumFetchOptions();
            list(
    $thread$forum) = $ftpHelper->assertThreadValidAndViewable($threadId$threadFetchOptions$forumFetchOptions);        
            
    $threadTitle $thread['title'];
     
  5. AndyB

    AndyB Well-Known Member

    That is so cool. now I can remove a bunch of code I used to verify view permissions using a query.
     
  6. Chris D

    Chris D XenForo Developer Staff Member

    People often forget:

    There's probably a few hundred thousand lines of code already written in the XenForo core. Always have the mentality of: "How does XenForo do that"? So sometimes although we KNOW how to check permissions using a query, it's often best to take a step back and think: "IS that how XenForo did it?". And this is a perfect example :)
     
    Marcus, Liam W, Jeremy and 1 other person like this.
  7. AndyB

    AndyB Well-Known Member

    Turns out that I need to check for view permissions in another part of my add-on. So I was wondering is there a more efficient way of getting the thread title without using the five lines of code as shown in post #4, the view permissions are not needed at this point.

    Thank you.
     
  8. Liam W

    Liam W Well-Known Member

    I always use this mentality - whenever I make an addon using a piece of code or area I haven't used before, I see how it is done in the core and try and replicate it as much as possible ;)

    Liam
     
  9. AndyB

    AndyB Well-Known Member

    Looks like the most efficient way to get the thread title in this case is to query the database. In my Thread.php file I added the following code:

    PHP:
    $threadTitle $this->getModelFromCache('Andy_SimilarThreads_Model')->getThreadTitle($threadId);
    and the Model.php file I added the following code:

    PHP:
      public function getThreadTitle($threadId)
       {
         
    // get thread title
         
    return $this->_getDb()->fetchOne("
           SELECT xf_thread.title
           FROM xf_thread
           WHERE xf_thread.thread_id = '
    $threadId'
         "
    );   
       }
     
  10. euantor

    euantor Well-Known Member

    Might be a good idea to parametrise that and to add a LIMIT 1 clause to it. Probably just personal preference though.
     
  11. AndyB

    AndyB Well-Known Member

    Thank you for your feedback, euantor.

    Could you give me an example of how to parameterize it.
     
  12. Daniel Hood

    Daniel Hood Well-Known Member

    Since it seems you're extending the controller, why not just use
    PHP:
    $parent parent::actionIndex(); $title $parent->params['thread']['title'];
    Maybe I'm missing something?

    Typed this from my phone, may not be accurate variable names and typos lol

    Oh and your query would be :
    Code:
    query("select * from xf_thread where thread_id = ?", array($threadid)) 
    you just replace your variables with ?s and then add them to an array in the second parameter.
     
    AndyB likes this.
  13. AndyB

    AndyB Well-Known Member

    Thank you for your assistance, Daniel.

    Indeed I'm extending a controller actionIndex(). I was able to get the following to work:

    PHP:
    $parent parent::actionIndex();

    $threadTitle $parent->_params['thread']['title'];
    But it wasn't reliable, I would get on occasion server errors in my server error log. So I decided to just use the query instead which works perfect all the time.
     
    Last edited: Oct 17, 2013
  14. AndyB

    AndyB Well-Known Member

    I'm still not up to speed writing queries as you describe.

    Isn't using the select * not optimal? I was under the impression that selecting only those field you need is a better way to go.
     
  15. Daniel Hood

    Daniel Hood Well-Known Member

    That's because it will only work if its an instance of response view. this should never error:
    PHP:
    if ($parent instanceof XenForo_ControllerResponse_View)
    $threadTitle =
    Replace ... with my original code.

    I see no reason to add an additional query.
    [Edit]
    Yeah replace the * with the fields you want to use. I'm being pretty lazy due to typing from my phone. Sorry
     
    euantor and AndyB like this.
  16. AndyB

    AndyB Well-Known Member

    This works perfect! Thank you, Daniel.

    PHP:
                $parent parent::actionIndex();
                
                if (
    $parent instanceof XenForo_ControllerResponse_View)
                {
                    
    $threadTitle $parent->params['thread']['title'];
                } 
     
    Last edited: Oct 17, 2013
  17. Daniel Hood

    Daniel Hood Well-Known Member

    No problem.

    Now that I'm on a computer, I'll explain the query part. I probably messed you up by using query() as the function, I was being lazy like I said since I was on my phone and auto correct makes typing code a pain.

    PHP:
    return $this->_getDb()->fetchOne("
     SELECT xf_thread.title
     FROM xf_thread
     WHERE xf_thread.thread_id = '
    $threadId'
     "
    );
    should be

    PHP:
    return $this->_getDb()->fetchOne("
     SELECT xf_thread.title
     FROM xf_thread
     WHERE xf_thread.thread_id = ?
     "
    , array ($threadId));
    With prepared statements you just replace all variable spots with a ? and then pass the replacements in an array as the second paramter of the query.
     
    AndyB likes this.
  18. Chris D

    Chris D XenForo Developer Staff Member

    Although not incorrect, you can simplify that code somewhat further:

    PHP:
    return $this->_getDb()->fetchOne('
        SELECT title
        FROM xf_thread
        WHERE thread_id = ?
    '
    $threadId);
    It's the same thing but:
    • The tables don't need prefixes. We're only dealing with one table.
    • The binding parameters do not need to be passed as an array if there's only a single parameter.
     
    AndyB likes this.
  19. Daniel Hood

    Daniel Hood Well-Known Member

    Good advice, I just figured I'd have him do the array so that down the road he's not like
    PHP:
    return $this->_getDb()->fetchOne(' select * from table where field1 = ? and field2 = ?'$field1$field2); 
    isn't working?

    Was just thinking that since he's "still not up to speed writing queries as you describe." I'd avoid the most simplified solution.
     
    Last edited: Oct 17, 2013
    Luke F, AndyB, euantor and 1 other person like this.
  20. AndyB

    AndyB Well-Known Member

    Daniel and Chris,

    Thank you both for the detailed explanation on proper query syntax.
     

Share This Page