How to get the thread title from Xenforo_ControllerPublic_Thread

[AndyB]

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.

 

[xf_phantom]

What are you trying to achieve?

Couldn't you call the parent method and extract the title from the returned value?
If you need it before you call the parent method, you'll need to fetch it yourself! (that's why i suggested this http://xenforo.com/community/thread...age-inside-the-controllers.49741/#post-532135 )

 

[Chris D]

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:

        $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]

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

This code works perfectly.

		// 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'];

 

[AndyB]

That makes sure the user can actually view the thread and forum.

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

 

[Chris D]

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

 

[AndyB]

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.

 

[Liam W]

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

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

 

[AndyB]

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:

$threadTitle = $this->getModelFromCache('Andy_SimilarThreads_Model')->getThreadTitle($threadId);

and the Model.php file I added the following code:

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

 

[euantor]

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

 

[AndyB]

Thank you for your feedback, euantor.

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

 

[Daniel Hood]

Since it seems you're extending the controller, why not just use

$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 :

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]

Thank you for your assistance, Daniel.

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

$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.

 

[AndyB]

Oh and your query would be :

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.

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.

 

[Daniel Hood]

That's because it will only work if its an instance of response view. this should never error:

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

 

[AndyB]

This works perfect! Thank you, Daniel.

			$parent = parent::actionIndex();
			
			if ($parent instanceof XenForo_ControllerResponse_View)
			{
				$threadTitle = $parent->params['thread']['title'];
			}

 

[Daniel Hood]

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.

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

should be

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.

 

[Chris D]

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

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.

 

[Daniel Hood]

Good advice, I just figured I'd have him do the array so that down the road he's not like

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.

 

[AndyB]

Daniel and Chris,

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