XF 2.0 Duplicate position in xf_post causes my add-on to show error

[AndyB]

If there's a duplicate position (see position 21) in the xf_post table like this:



and in the thread_view there's a page number showing which doesn't actually exist, if that page number is clicked my add-on shows an error.

I was hoping the redirect portion of the PHP code would prevent the error but it does not.

<?php

namespace Andy\Calendar\XF\Pub\Controller;

use XF\Mvc\ParameterBag;

class Thread extends XFCP_Thread
{
    public function actionIndex(ParameterBag $params)
    {     
        //########################################
        // show calendar dates in thread_view
        //########################################
     
        // get thread
        $thread = $this->assertViewableThread($params->thread_id, $this->getThreadViewExtraWith());

        // check if redirect
        if ($thread->discussion_type == 'redirect')
        {
            if (!$thread->Redirect)
            {
                return $this->noPermission();
            }

            return $this->redirectPermanently($this->request->convertToAbsoluteUri($thread->Redirect->target_url));
        }     
...

This is the error message:



Line #139



How can I prevent this error message?

Thank you.

 

[Chris D]

The error message suggests this is a problem with your extension of the code. Notably, you're trying to set view params on a Redirect reply rather than a View reply.

There are many issues in your initial snippet of code which I believe you have worked through before for other controllers/add-ons.

 

[Snog]

The other question to ask is how did a duplicate position number get into the table? That shouldn't happen.

 

[Chris D]

The first question to ask is how did a duplicate position number get into the table? That shouldn't happen.

I wouldn't necessarily focus on that for now. It's not even relevant to the error being reported by the code.

 

[AndyB]

There are many issues in your initial snippet of code which I believe you have worked through before for other controllers/add-ons.

Yes indeed I have, but that was for XF1. Here's an example:

        // return parent action if this is a redirect or other non View response
        if (!$parent instanceof XenForo_ControllerResponse_View)
        {
            return $parent;
        }

So what should I use in XF2?

 

[Chris D]

In XF2, the equivalent to the View "ControllerResponse" is the View "Reply".

        // return parent action if this is a redirect or other non View response
        if (!$parent instanceof \XF\Mvc\Reply\View)
        {
            return $parent;
        }

 

[AndyB]

Thank you, Chris.

My add-on has been updated and now has this in the beginning:

<?php

namespace Andy\Calendar\XF\Pub\Controller;

use XF\Mvc\ParameterBag;

class Thread extends XFCP_Thread
{
	public function actionIndex(ParameterBag $params)
	{		
		//########################################
		// show calendar dates in thread_view
		//########################################
		
		// get parent		
		$parent = parent::actionIndex($params);

        // return parent action if this is a redirect or other non View response 
        if (!$parent instanceof \XF\Mvc\Reply\View)
        {
            return $parent;
        }	
		
		// get visitor
		$visitor = \XF::visitor();		

		// check for user group permission
		if (!$visitor->hasPermission('calendar', 'view'))
		{
			return $parent;
		}
		
		// get thread
		$thread = $this->assertViewableThread($params->thread_id, $this->getThreadViewExtraWith());
...

Does this look correct?

 

[Chris D]

Yes, but similar advice from previous threads still stands.

Do you need to query for the thread again? Does the original view reply contain a thread in its params? Can you use that instead?

 

[AndyB]

Do you need to query for the thread again? Does the original view reply contain a thread in its params? Can you use that instead?

Thank you for pointing that out. I'll replace with:

        // get threadId
        $threadId = $params->thread_id;

 

[Jake B.]

Thank you for pointing that out. I'll replace with:

        // get threadId
        $threadId = $params->thread_id;

I don't think that's what he was referring to, how is $thread being defined now?

 

[Snog]

@AndyB look at getParam for the parent. $thread should already exist in the view, so there's no need to query for it again unless you're changing the contents of it.

 

[AndyB]

Hi Jake and Snog,

Sorry I'm not understanding. Please elaborate.

I currently have:

<?php

namespace Andy\Calendar\XF\Pub\Controller;

use XF\Mvc\ParameterBag;

class Thread extends XFCP_Thread
{
    public function actionIndex(ParameterBag $params)
    {        
        //########################################
        // show calendar dates in thread_view
        //########################################
        
        // get parent        
        $parent = parent::actionIndex($params);

        // return parent action if this is a redirect or other non View response 
        if (!$parent instanceof \XF\Mvc\Reply\View)
        {
            return $parent;
        }    
        
        // get visitor
        $visitor = \XF::visitor();        

        // check for user group permission
        if (!$visitor->hasPermission('calendar', 'view'))
        {
            return $parent;
        }
        
        // get threadId
        $threadId = $params->thread_id;

Thank you.

 

[Snog]

Yes, but similar advice from previous threads still stands.

Do you need to query for the thread again? Does the original view reply contain a thread in its params? Can you use that instead?

This is the part in the code you originally posted that's being questioned...

$thread = $this->assertViewableThread($params->thread_id, $this->getThreadViewExtraWith());

The thread should already be available in the parent with $parent->getParam('thread');

 

[Jake B.]

Are you using anything from the thread table other than thread_id? Surely would be easier if whatever you're loading used a relation to the thread so you could just do $thread->Data

Also, should probably prefix at the permission group with something as to not cause issues if XF ever released a calendar feature

 

[AndyB]

This is the part in the code you originally posted that's being questioned...
The thread should already be available in the parent with $parent->getParam('thread');

If I try that code I get the following error.

 

[AndyB]

Are you using anything from the thread table other than thread_id? Surely would be easier if whatever you're loading used a relation to the thread so you could just do $thread->Data

I get the same error as pointed out in the previous post Undefined variable.

 

[Snog]

If I try that code I get the following error.

View attachment 160903

Then evidently it's not in the parent for some reason.

If it were there, then this would display something.

$parent = parent::actionIndex($params);
print_r($parent->getParam('thread'));
die();

 

[Chris D]

If I try that code I get the following error.

View attachment 160903

Then evidently it's not in the parent for some reason.

If it were there, then this would display something.

$parent = parent::actionIndex($params);
print_r($parent->getParam('thread'));
die();

That error is certainly not caused by $parent->getParam('thread'). It's caused by using a variable called $thread which is not defined.

 

[Jake B.]

I get the same error as pointed out in the previous post Undefined variable.

$thread will of course have to be defined first, and Data was just an example, you'll have to create the actual relation before that'll work as well

 

[AndyB]

So if I assume correctly in this case using the following is the best way:

$threadId = $params->thread_id;