Why doesn't this work?

shawn

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.
 
Chris Deeming said:
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.
Click to expand...

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'");
 
Syndol said:
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'");
Click to expand...
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.
 
Syndol said:
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'");
Click to expand...

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)
 
Chris Deeming said:
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)
Click to expand...

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

shawn said:
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);
            }       
        }
    }
Click to expand...

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

shawn said:
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='';
            }       
        }
    }
Click to expand...
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.
 
Chris Deeming said:
I'm 90% sure the "removeUserGroupChange" function doesn't do what you think it does. Let's go back to the previous post...
Click to expand...

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

Chris Deeming said:
The use of undefined constant is from this line:

PHP: 
$memberGroupArray = explode(',', $user[secondary_group_ids]);
Click to expand...

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.
 
shawn said:
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.
Click to expand...

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.
 
You must log in or register to reply here.

Similar threads

Mr Lucky
  • Question Question
XF 1.5 Why doesn't this template modification work?
Replies
7
Views
1K
Mr Lucky
Mr Lucky
Marcel
Cron job to move threads...Works with all prefixes except one?
Replies
6
Views
2K
Amin Sabet
Amin Sabet
Back
Top Bottom