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.Problem/Motivation
Hard-coding to an SQL backend is bad mk
Proposed resolution
Use the entity API
Remaining tasks
Reviews
User interface changes
None
API changes
None
Data model changes
None
| Comment | File | Size | Author |
|---|---|---|---|
| #24 | interdiff.23-24.txt | 917 bytes | Spokje |
| #24 | 2569147-24.patch | 1 KB | Spokje |
| |||
Issue fork drupal-2569147
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #4
olli CreditAttribution: olli commentedRebased and fixed TrackerUserUidTest.
I'm not sure if it is possible to remove this->database from the query() method but I think user->getUsername() is better than a db query.
Comment #16
smustgrave CreditAttribution: smustgrave at Mobomo commentedComment #17
Medha KumariRerolled the patch #4 in Drupal 9.5.x.
Comment #18
Medha KumariRerolled patch #17 in Drupal 9.5.x. and fixed custom command failed
Comment #20
Medha KumariFixing Error in #18
Comment #21
Akram KhanFix CCF #20
Comment #23
Ankit.Gupta CreditAttribution: Ankit.Gupta at Dotsquares Ltd. commentedComment #24
SpokjeComment #26
smustgrave CreditAttribution: smustgrave at Mobomo commentedThis issue is being reviewed by the kind folks in Slack, #need-reveiw-queue. We are working to keep the size of Needs Review queue [2700+ issues] to around 200, following Review a patch or merge require as a guide.
Thank you @Spokje for fixing up that patch.
Change looks good to me and see it passes 9.5, 10.0, and 10.1 (so much green!)
Comment #27
quietone CreditAttribution: quietone commentedI talked to larowlan about this and we agree that this can use the null safe operator.
$title = $user?->getDisplayName();.Also, this is a 10.1.x only change. Therefor I have changed the version.
This is suitable for a novice, adding tag.
Comment #30
SpokjeComment #31
smustgrave CreditAttribution: smustgrave at Mobomo commentedOnly open thread
You mentioned it may not be worth the effort and for a small change that path would require a change record, trigger_error (I would imagine), and then tests.
Just my thought.
Comment #32
alexpottThis is actually a bug fix as it is correctly changing the title from the user's account to the display name.
I wonder what this change means for argument's summary name field... in UserViewsData we do:
But now this argument is using the display name which might have nothing to do with account name. I think we need to address this inconsistency if we change to display name here. FWIW off the top of my head I have no idea what summary queries do :)
Comment #34
sahil.goyal CreditAttribution: sahil.goyal as a volunteer and at Srijan | A Material+ Company for Drupal India Association commentedUpdated the plugin to use dependency injection for the entity-type manager instead of the global singleton
\Drupal\user\Entity\User::load(). Instantiate the plugin with the entity-type manager, and modified the execute() method to load the user entity using the injected entity-type manager.Comment #35
sahil.goyal CreditAttribution: sahil.goyal as a volunteer and at Srijan | A Material+ Company for Drupal India Association commentedComment #36
smustgrave CreditAttribution: smustgrave at Mobomo commentedNow I believe this will need to default to NULL and trigger_error when the Entity Type Manager isn't passed in.
Should have test coverage I believe too.
#32 still needs an answer also.