New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
13976 segments not deleted #17231
13976 segments not deleted #17231
Conversation
Hi all, this PR is not "finished" yet, I'd like to add an extra test, any suggestion is appreciated. It's happening in Thanks |
Hi @flamisz if you wanted to test that it's actually called you could basically first check there are segments for that user like |
Code looks good, but it appears to be causing another integration test to fail. |
@diosmosis the test had errors in the (of course the error was caused by my pr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, will merge tomorrow after a bit of time's passed since the 4.2.0 release
$this->transferAllUserSegmentsToSuperUser($userLogin); | ||
} | ||
|
||
public static function transferAllUserSegmentsToSuperUser($userLogin) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, noticed one other issue: is there a reason this method is static
? If not let me know and I'll change it before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
definitely NOT, I'm not sure why it is static, sorry. thanks for changing it @diosmosis
Description:
fix for #13976
Review