Not a bug Problem in UserGroupPromotion

MIXER

Member
Hello, in my mind I had found a problem with UserGroupPromotion system.
On my forum I have 1 promote condition and after runnig cron job XenForo_CronEntry_UserGroupPromotion:runPromotions I have problem, user are promoted to a new group, I watched it in his member profile in admin panel, and inserted new sql record in xf_user_group_promotion_log, but after running cron job again, this user are removes from a group and his record in xf_user_group_promotion_log deleted too.

I found bug in 292 line of library/XenForo/Model/UserGroupPromotion.php file. In my mind it has wrong logic with {} construction. I tested cron job with my change, and I guess it work correctly.

Plese check this:

Original version:
PHP:
if (XenForo_Helper_Criteria::userMatchesCriteria($promotion['user_criteria'], false, $user))
            {
                if (!$hasPromotion)
                {
                    $this->promoteUser($promotion, $user['user_id']);
                    $changes++;
                }
            }
            else if ($hasPromotion)
            {
                $this->demoteUser($promotion, $user['user_id']);
                $changes++;
            }
My version:
PHP:
if (XenForo_Helper_Criteria::userMatchesCriteria($promotion['user_criteria'], false, $user))
            {
                if (!$hasPromotion)
                {
                    $this->promoteUser($promotion, $user['user_id']);
                    $changes++;
                }
                else if ($hasPromotion)
                {
                    $this->demoteUser($promotion, $user['user_id']);
                    $changes++;
                }
            }
Please confirm this changes if it correct for your mind.
 

Mike

XenForo developer
Staff member
If you find that people are being removed after they get the promotion, then that would point to an issue with the promotion criteria. Presumably your promotion criteria is that they are not a member of the group you added them to.

Your suggested code change is not correct. The original code is correct: any user who does not match the criteria who has the promotion should be demoted; a promotion is only valid while they meet the criteria. I'm pretty sure your change would actually "ping pong" users while they meet the criteria and never actually demote people that no longer meet the criteria.
 

MIXER

Member
I not agree with your response.
Please explain me how user are match promotion criteria and doesn't match it in 1 minute? I started cron job in 1-2 minutes about 3 times on test user and he is promoted and demoted every time, then I started job...

Here is my promotion's criteries:
upload_2016-12-1_10-9-16.png



upload_2016-12-1_10-10-6.png



I shooted only options, that I changed in promotion's criteria...
 

Martok

Well-known member
The problem is in your second screenshot.

For ease of understanding to all, I'll simplify the names of the user groups.

Your promotion is to add a user to User group B.
In your criteria, you have said that they must be in User group A but not in User group B for the promotion to apply.

When the user group promotion runs, a user who is in User group A but not in User group B and meets all the other criteria is then promoted and is added to User group B. However, that user is then both in User group A and B. This means that they no longer meet the user group promotion criteria (which explicitly says that for the promotion to apply, they should not be in User group B). As the user group promotion criteria is no longer met, the user is removed from User group B.

Due to this, the promotion and demotion cycle will occur each time the user group promotion cron runs - each time the user will be promoted to User group B (as they meet the criteria) and then removed from User group B (as they no longer meet the criteria).


The solution is very easy. Simply remove the criteria "User is not a member of User group B" and it will work. When the cron runs, your user will be promoted to User group B and will remain there as they continue to meet the criteria.
 
Top