Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
The documentation of message_create()
states that the $account
parameter is ignored when the $args['uid']
parameter is set, as is the case in oa_messages_create()
. As such, the call to user_load
can be removed.
Comment | File | Size | Author |
---|---|---|---|
#3 | 2868908-oa_messages-removeuserload-3.patch | 750 bytes | Jorrit |
Comments
Comment #2
Jorrit CreditAttribution: Jorrit at nCode for DOM Digital Online Media GmbH commentedComment #3
Jorrit CreditAttribution: Jorrit at nCode for DOM Digital Online Media GmbH commentedFix the patch.
Comment #4
mpotter CreditAttribution: mpotter at Phase2 commentedLooking at the OG message_create code, I don't see any evidence of this. It is setting the $values['user'] field with the account and not overriding the 'uid' value. I would need to see a set of test cases that show this is not an issue. The chance of this breaking something obscure seems too high.
Keep in mind that messages get created by things like the oa_event_import when handling incoming comment replies, etc which are tricky to test. At some point, even with the 'uid' field it's going to need to do a user_load which should cache the result, so calling it a second time shouldn't be a performance issue.
Unless there is an actual bug being fixed here I am leery to make this kind of change.
Comment #5
Jorrit CreditAttribution: Jorrit at nCode for DOM Digital Online Media GmbH commentedI understand that you might not want to merge this. The fact that message_create sets $account is a bug on their end, because they ignore the value later on in
Message::__construct()
:In the case of
oa_messages_create
$values['uid']
is set, so$values['user']
is ignored.In my case I want to use
oa_messages_create()
to create several extra notifications for users that are not necessarily loaded at that time and I want to make it as efficient as possible. I'll leave it up to you to decide if this is worth considering.Comment #6
mpotter CreditAttribution: mpotter at Phase2 commentedAhh, yes, you are correct about the logic in the message constructor. That is the code I needed to see.
I can see the use-case for messages from users that are not loaded. Can you confirm that when this patch is used that the user is still not loaded somewhere else? Seems like it would need the full user object in case the message text references the user (like to expand the user's name in the notification subject)
Comment #7
Jorrit CreditAttribution: Jorrit at nCode for DOM Digital Online Media GmbH commentedYou're right, in most realistic scenarios user_load() will be called somewhere in the same page load for the user id. As you noticed, I am working with message and oa_messages today and I noticed that both a uid and user were passed to message_create. Because it wasn't costing me a lot of time to work out a patch I submitted one.
My use-case is that I am building an event invite system where entire groups can be invited. Each user will get a personal invitation, which doesn't contain any user details, as the message is something like 'You've been invited to event XXXX'.