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

Why doesn't this work?

shawn

Well-known member
#1
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();
                }
 
            }
 

Chris D

XenForo developer
Staff member
#2
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.
 

shawn

Well-known member
#3
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....
 
S

Syndol

Guest
#4
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'");
 

Chris D

XenForo developer
Staff member
#5
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
 

Chris D

XenForo developer
Staff member
#6
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.
 

shawn

Well-known member
#7
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.
 

Chris D

XenForo developer
Staff member
#8
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.
 

shawn

Well-known member
#11
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.
 

Chris D

XenForo developer
Staff member
#12
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)
 

shawn

Well-known member
#13
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.
 

shawn

Well-known member
#14
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='';
            }         
        }
    }
 

shawn

Well-known member
#15
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);
            }         
        }
    }
 

Chris D

XenForo developer
Staff member
#17
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.
 

shawn

Well-known member
#18
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.
 

shawn

Well-known member
#19
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.
 

Chris D

XenForo developer
Staff member
#20
Probably should be:

AND node_id IN (?)

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