Why doesn't this work?

shawn

Well-known member
It's supposed to be fetching the soft deleted threads and changing their node, but it's fetching everything instead.

Code:
            $threadModel = XenForo_Model::create('XenForo_Model_Thread');
            $conditions = array(
                'discussion_state' => 'deleted',
                'forum_id'    => $sourceForum,
            );
       
            $threads = $threadModel->getThreads($conditions, $fetchOptions);
 
            if ($threads)
            {
                foreach ($threads as $thread)
                {
                    $dw = XenForo_DataWriter::create('XenForo_DataWriter_Discussion_Thread');
                    $dw->setExistingData($thread);
                    $dw->set('node_id', $options->xefDestinationForum);
                    $dw->save();
                }
 
            }
 
Should be:

PHP:
$conditions = array(
	'deleted' => true,
	'node_id' => $sourceForum
);

Technically forum_id will work, but node_id is the criteria it looks at.

The function that does all the filtering based on conditions is "prepareThreadConditions" in the thread model.
 
Should be:

PHP:
$conditions = array(
'deleted' => true,
'node_id' => $sourceForum
);

Technically forum_id will work, but node_id is the criteria it looks at.

The function that does all the filtering based on conditions is "prepareThreadConditions" in the thread model.

Nope... no joy yet. It still moves everything in the node.

But to your point, this would probably be a whole lot quicker with something resembling a proper dev environment....
 
You will need to create your own function to fetch deleted threads as the function that processes the conditions for getThreads() automatically adds visible threads.
See function prepareStateLimitFromConditions() in Model.php line 416:
$states = array("'visible'");
 
You will need to create your own function to fetch deleted threads as the function that processes the conditions for getThreads() automatically adds visible threads.
See function prepareStateLimitFromConditions() in Model.php line 416:
$states = array("'visible'");
Just noticed that myself :D
 
Hmm, rather than writing a whole new function, you could simply:

PHP:
$threads = $threadModel->getThreads($conditions, $fetchOptions);
if ($threads)
{
	foreach ($threads as $thread)
	{
		if ($thread['discussion_state'] == 'deleted')
		{
			$dw = XenForo_DataWriter::create('XenForo_DataWriter_Discussion_Thread');
			$dw->setExistingData($thread);
			$dw->set('node_id', $options->xefDestinationForum);
			$dw->save();
		}
	}
}

It's obviously not as optimal, but it depends how many threads you're dealing with.
 
You will need to create your own function to fetch deleted threads as the function that processes the conditions for getThreads() automatically adds visible threads.
See function prepareStateLimitFromConditions() in Model.php line 416:
$states = array("'visible'");

Oops.

Chris's solution works, though... and for a midnight cron job that moves deleted threads out of the classified ads... probably not terrible. Worst case, it might churn through a few thousand lines...?

Thanks for all of the help.
 
Well, yeah it works.

But here, so does this :)

PHP:
		$db = XenForo_Application::getDb();
		
		$threads = $db->fetchAll('
			SELECT *
			FROM xf_thread
			WHERE discussion_state = ?
			AND node_id = ?
		', array('deleted', $sourceForum));
		
		if ($threads)
		{
			foreach ($threads AS $thread)
			{
				$dw = XenForo_DataWriter::create('XenForo_DataWriter_Discussion_Thread');
				$dw->setExistingData($thread);
				$dw->set('node_id', $options->xefDestinationForum);
				$dw->save();
			}			
		}

Yeah it's not very MVC-like but at least it's only loading the threads you need.
 
I think it ought to work.... except that $sourceForum is actually an array. I tried it and got an array to string conversion error on that line. I think that's what's screwing it up.
 
Yeah that won't work as it is but you can do that.

The where clause needs to be slightly different.
On phone now but something like:

and node_id in ($sourceForum)
 
Yeah that won't work as it is but you can do that.

The where clause needs to be slightly different.
On phone now but something like:

and node_id in ($sourceForum)

What I got to work was more like this:

Code:
        $threads = $db->fetchAll('
            SELECT *
            FROM xf_thread
            WHERE discussion_state = "deleted"
            AND node_id = (?)
        ', $sourceForum);

Everything else I tried threw an 'array to string conversion' or 'unknown column array in where clause' error. But it's happy with it like that.
 
I like this kludge so much, I decided to try to put it to other uses. Another cron script this time. Check to see if a user is in secondary user group 'A' and not in group 'B'. If so, remove them from group A. Basically, this is cleaning up some old vB remnants. It needs to run nightly because group B is tied to an expiring account upgrade.

The problem I'm having now is with the line "$memberGroupArray = explode(',', $user[secondary_group_ids]);" It throws a "use of undefined constant" error for 'secondary_group_ids', presumably because I'm not referring to the user data array correctly.

Suggestions?

Also, do I really need to redefine the variables after each user? That was in the original VB script, but I think it's unnecessary.


Code:
    {
        $db = XenForo_Application::getDb();
     
        $users = $db->fetchAll('
            SELECT *
            FROM xf_user
            WHERE find_in_set(12,secondary_group_ids)
            AND !find_in_set(9,secondary_group_ids)
        ');
     
        if ($users)
        {
            foreach ($users AS $user)
            {
                $memberGroupArray = explode(',', $user[secondary_group_ids]);
                $memberGroupArray = array_diff ( $memberGroupArray, array(12) );
                $secondaryGroupIds = implode(',', $memberGroupArray);
 
                $dw = XenForo_DataWriter::create('XenForo_DataWriter_User');
                $dw->setExistingData($user);
                $dw->setSecondaryGroups($secondaryGroupIds);
                $dw->save();
 
                $memberGroupArray='';  //reset array for the next user?
                $secondaryGroupIds='';
            }         
        }
    }
 
I found the removeUserGroupChange function, which looks handy. Came up with plan B. It looks much better, but doesn't run:

Code:
    {
        $db = XenForo_Application::getDb();
    $demotionModel = XenForo_Model::create('XenForo_Model_User');
     
        $users = $db->fetchAll('
            SELECT *
            FROM xf_user
            WHERE find_in_set(12,secondary_group_ids)
            AND !find_in_set(9,secondary_group_ids)
        ');
     
        if ($users)
        {
            foreach ($users AS $userId => $user)
            {
                $demotionModel->removeUserGroupChange($userId,12);
            }         
        }
    }
 
Sorry about the delay... Last post first...

I found the removeUserGroupChange function, which looks handy. Came up with plan B. It looks much better, but doesn't run:

Code:
    {
        $db = XenForo_Application::getDb();
    $demotionModel = XenForo_Model::create('XenForo_Model_User');
   
        $users = $db->fetchAll('
            SELECT *
            FROM xf_user
            WHERE find_in_set(12,secondary_group_ids)
            AND !find_in_set(9,secondary_group_ids)
        ');
   
        if ($users)
        {
            foreach ($users AS $userId => $user)
            {
                $demotionModel->removeUserGroupChange($userId,12);
            }       
        }
    }

I'm 90% sure the "removeUserGroupChange" function doesn't do what you think it does. Let's go back to the previous post...

I like this kludge so much, I decided to try to put it to other uses. Another cron script this time. Check to see if a user is in secondary user group 'A' and not in group 'B'. If so, remove them from group A. Basically, this is cleaning up some old vB remnants. It needs to run nightly because group B is tied to an expiring account upgrade.

The problem I'm having now is with the line "$memberGroupArray = explode(',', $user[secondary_group_ids]);" It throws a "use of undefined constant" error for 'secondary_group_ids', presumably because I'm not referring to the user data array correctly.

Suggestions?

Also, do I really need to redefine the variables after each user? That was in the original VB script, but I think it's unnecessary.


Code:
    {
        $db = XenForo_Application::getDb();
   
        $users = $db->fetchAll('
            SELECT *
            FROM xf_user
            WHERE find_in_set(12,secondary_group_ids)
            AND !find_in_set(9,secondary_group_ids)
        ');
   
        if ($users)
        {
            foreach ($users AS $user)
            {
                $memberGroupArray = explode(',', $user[secondary_group_ids]);
                $memberGroupArray = array_diff ( $memberGroupArray, array(12) );
                $secondaryGroupIds = implode(',', $memberGroupArray);
 
                $dw = XenForo_DataWriter::create('XenForo_DataWriter_User');
                $dw->setExistingData($user);
                $dw->setSecondaryGroups($secondaryGroupIds);
                $dw->save();
 
                $memberGroupArray='';  //reset array for the next user?
                $secondaryGroupIds='';
            }       
        }
    }
The use of undefined constant is from this line:

PHP:
$memberGroupArray = explode(',', $user[secondary_group_ids]);

secondary_group_ids is being treated as a constant because it isn't wrapped in single quotes, so the line should be:

PHP:
$memberGroupArray = explode(',', $user['secondary_group_ids']);

The arrays do not need to be reset for the next iteration because they are redefined at the start of the iteration so effectively they are overwritten each time making it unnecessary to clear them.

So that should help.
 
I'm 90% sure the "removeUserGroupChange" function doesn't do what you think it does. Let's go back to the previous post...

You're absolutely correct. I just finished up reading this thread, where that same topic came up.

The use of undefined constant is from this line:

PHP:
$memberGroupArray = explode(',', $user[secondary_group_ids]);

Ugh... thank you.

The only other problem I had was that I was returning the secondary user groups as a string, when the Datawriter wanted an array. So here's the final, corrected script:

PHP:
    {
        $db = XenForo_Application::getDb();
   
        $users = $db->fetchAll('
            SELECT *
            FROM xf_user
            WHERE find_in_set(12,secondary_group_ids)
            AND !find_in_set(9,secondary_group_ids)
        ');
   
        if ($users)
        {
            foreach ($users AS $user)
            {
                $memberGroupArray = explode(',', $user['secondary_group_ids']);
                $memberGroupArray = array_diff ( $memberGroupArray, array(12) );
 
                $dw = XenForo_DataWriter::create('XenForo_DataWriter_User');
                $dw->setExistingData($user);
                $dw->setSecondaryGroups($memberGroupArray);
                $dw->save();
            }       
        }
    }

Thanks for your help.
 
What I got to work was more like this:

Code:
        $threads = $db->fetchAll('
            SELECT *
            FROM xf_thread
            WHERE discussion_state = "deleted"
            AND node_id = (?)
        ', $sourceForum);

Everything else I tried threw an 'array to string conversion' or 'unknown column array in where clause' error. But it's happy with it like that.

Ugh.... back to this.

I uploaded this to the live server only to discover that this query doesn't work if I have more than one forum selected in the options. It throws a "Number of variables doesn't match number of parameters in prepared statement" error.
 
Probably should be:

AND node_id IN (?)

Make sure $sourceForum is always an array, even if it only contains a single node_id.
 
Top Bottom