Code comments still mention removed function user_save()

Thanks for filing this... Someone needs to figure out what function would be appropriate to mention in the documentation -- I mean if we have a test called UserSaveTest.php, what function is it testing? And where are the hooks in user.api.php that previously mentioned user_save() being invoked now, if anywhere -- do they need to be removed because the hooks don't get invoked any more, or do they need their documentation updated?

Updated issue summary.

Documentation updated.

This docs are wrong. Because user_insert is gone too. The equivalent to user_save() is \Drupal\user\UserStorageController::save(). I am not sure how to document this.

You can just put \Drupal\user\UserStorageController::save() into the documentation.

Ok, you mean also hook_user_insert and _save is gone.
And it's better to say for hook_user_insert \Drupal\user\UserStorageController::create()?

No, hook_user_update/insert() still exist. But there never was "user_insert()" or "user_update()".

I think this should actually refer to \Drupal\Core\Entity\DatabaseStorageContoller::save(), that's where the actual logic is that this documentation is talking about. What you're calling in 99% of the cases is $user->save(), but that just forwards to the controller.

I think what we should do sometime next year is make sure that the generic hook_entity_*() crud hooks come with good documentation and then just make hook_user/term/node/comment..._*() simple one line docs with a @see and maybe type specific information but the point of the whole entity crud unification is to have as less entity-type-specific stuff as possible. This information for example, applies to every entity that uses the default controller/one that uses transactions.

RE #6 @berdir - see #1824184: Remove and replace entity type specific API hook documentation with generic entity API hook documentation

Ok, I added the reference to \Drupal\user\UserStorageController::save().

Thanks!

But... This patch has in it:

- * transaction is not finalized until the save operation is entirely completed
- * and user_save() goes out of scope. You should not rely on data in the
+ * transaction is not finalized until the update operation is entirely completed
+ * and hook_user_update() goes out of scope. You should not rely on data in the

That changes user_save() to hook_user_update() as the function that has to go out of scope to finalize the transaction. I don't think that's accurate -- isn't it UserStorageController::save() that has to go out of scope?

Actually, it looks like it is really DatabaseStorageController::save() that has to go out of scope, since that is what starts the transaction. Probably that is the function that should be mentioned in the other changes in this patch too?

Thanks for review.

If I wrong in my adoption, please let me know.

A small example. If I call $acount->save() that the function calls UserStorageController::save() and this is a impl. of an Interface and on the other hand this function calls DatabaseStorageController::save().

I think the UserStorageController::save() goes out of scope. And DatabaseStorageController::save() must mentioned to check it?

UserStorageController::save() overwrites DatabaseStorageController::save() to do something additional, what it does is not relevant for that comment. What that comment is talking about happens in DataStorageController::save() so that's the method that needs to be referenced there.

Right. The transaction is finalized when DatabaseStorageController::save() goes out of scope. That is what the comments need to say.

Updated as requested.

Take a look at the patch file. The first changed area in each function still has incorrect information about which function is going out of scope.

Sorry for that. New try.

I don't know what it didn't apply. BTW, found another reference in the test.

Assuming the test bot agrees, I agree with marking this RTBC now. Thanks for finding that additional spot to fix! I'll get this committed sometime in the next few days.

@penyaskito, thanks for update the patch. I was out of house today.

Thanks! This is committed to 8.x.