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

Discussion in 'XenForo Development Discussions' started by shawn, Jan 21, 2013.

  1. 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();
                    }
     
                }
     
  2. Chris D

    Chris D XenForo Developer Staff Member

    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 likes this.
  3. shawn

    shawn Well-Known Member

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

    Syndol Guest

    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 likes this.
  5. Chris D

    Chris D XenForo Developer Staff Member

    Just noticed that myself :D
     
    shawn likes this.
  6. Chris D

    Chris D XenForo Developer Staff Member

    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 likes this.
  7. shawn

    shawn Well-Known Member

    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.
     
  8. Chris D

    Chris D XenForo Developer Staff Member

    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 likes this.
  9. shawn

    shawn Well-Known Member

    I'm sure it won't be the first or last time that "standards" fall to the wayside in favor of laziness and efficiency. ;)
     
    Luke F and Chris D like this.
  10. Chris D

    Chris D XenForo Developer Staff Member

    Let us know how that works for you.

    I haven't tested it but it should work.
     
    shawn likes this.
  11. shawn

    shawn Well-Known Member

    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.
     
  12. Chris D

    Chris D XenForo Developer Staff Member

    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)
     
  13. shawn

    shawn Well-Known Member

    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.
     
    Chris D likes this.
  14. shawn

    shawn Well-Known Member

    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='';
                }         
            }
        }
     
  15. shawn

    shawn Well-Known Member

    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);
                }         
            }
        }
     
  16. shawn

    shawn Well-Known Member

    Bueller?

    Snarky comments also welcome....
     
  17. Chris D

    Chris D XenForo Developer Staff Member

    Sorry about the delay... Last post first...

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

    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 likes this.
  18. shawn

    shawn Well-Known Member

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

    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.
     
    Chris D likes this.
  19. shawn

    shawn Well-Known Member

    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.
     
  20. Chris D

    Chris D XenForo Developer Staff Member

    Probably should be:

    AND node_id IN (?)

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

Share This Page