Details
-
Type:
Bug
-
Status: Closed
-
Priority:
Minor
-
Resolution: Fixed
-
Affects Version/s: 2.5.4, 2.6.1, 2.7
-
Component/s: Database SQL/XMLDB, My home
-
Labels:
Description
Removal of all block instances on user pages is being handled by delete_context(CONTEXT_USER, $user->id) when user is deleted. However, if a user had a customised "My Home" page, a record will still remains in database.
It looks like all we need to do is add:
$DB->delete_records('my_pages', array('userid' => $user->id, 'private' => 1));
to delete_user() method. We will submit proposed fix in a bit.
Gliffy Diagrams
Activity
Hi Yuliya,
Thank you for providing patch for this issue. The patch looks great.
Additionally, it might be good to upgrade the table data by deleting rows of previously deleted user.
Please let me know if you need assistance to push this issue for integration.
Yep, can do that. Was wondering if you needed fixing that too. Will upload additional fix today.
Hi Rossiani,
Updated branches. Can you please have a look again if everything is in place and I will submit for integration.
Thanks!
Yuliya
Results for MDL-43797
- Remote repository: https://github.com/totara/moodle
- Remote branch m25_
MDL-43797to be integrated into upstream MOODLE_25_STABLE
- Remote branch m26_
MDL-43797to be integrated into upstream MOODLE_26_STABLE
- Remote branch master_
MDL-43797to be integrated into upstream master
Hi Yuliya,
Patch looks fine.
Please feel free to submit it for integration review.
Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!
Sorry, I'm reopening this. I think the upgrade code is really wrong:
1) Is it deleting the records for all the users with deleted = 0 ? Seems those are the active ones, not the deleted ones.
2) No way that IN construction can work against million of users, not we can iterate over all them 1 by 1. Surely it can be executed with a raw delete + select sql, not needed to load/prepare anything in the php side. Something like this DELETE + SELECT may do the trick 100% in the SQL side. OR here there is another example. Those are the preferred way to delete orphaned data.
Ciao 
Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
Hi Eloy,
It selects correct users because the last parameter in get_in_or_equal() is false, but I can see that you mean by optimizing the code. We will take a look again. Thanks for the feedback!
Yuliya
Thanks! (didn't realise the false)
Branches updated and rebased. Ready to be reviewed when you have time.
Thanks!
Hi Yuliya,
from memory, I don't remember if it's MySQL or Postgres (I think it's MySQL), but it sounds to me a lot that aliases are NOT allowed for the DELETE clause. So, in order to make the SQL 100% cross-db, you should replace all those "p." by "{my_pages}." and delete the alias from the DELETE clause. The second example linked above shows it.
Edited: s/now/NOT
Probably MySQL, because we tested it on Postgres.
(yes, confirmed, mysql breaks with: "ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'p ....')
Thanks! Just updated commits, so should be better now.
Note the the "user => u" alias was ok, afaik, it's a normal SELECT, but it doesn't harm to have everything un-aliased... going to put this again in the integration bag, thanks!
Thanks, Eloy! It's always good to learn something new
Integrated (25, 26 & master), thanks!
Test result: 
Success!
Tested in 2.5, 2.6 and master. Tested using PostgreSQL.
My page records are removed for deleted users.
Thanks for your hard work. Your code has now become a part of Moodle!
Results for
MDL-43797MDL-43797to be integrated into upstream MOODLE_25_STABLEMDL-43797to be integrated into upstream MOODLE_26_STABLEMDL-43797to be integrated into upstream master