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. $base_table = $info->getBaseTable();
if ($flag_type_def['entity_type'] == 'node') {
$base_table = $info->getDataTable();
}
Any entity might have a data table.
Ideally, we should have this logic taken care of by core: #2491687: Make EntityViewsData::getViewsTableForEntityType() public rather than protected.
In the meantime, we should fix this here anyway, and just use the data table if the entity type has one.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | special_casing_for-2491879-13.patch | 2.19 KB | sumitmadan |
| #11 | special_casing_for-2491879-11.patch | 2.19 KB | sumitmadan |
| #8 | special_casing_for-2491879-8.patch | 1.8 KB | sumitmadan |
| #2 | special_casing_for-2491879-2.patch | 580 bytes | sumitmadan |
Comments
Comment #1
joachim CreditAttribution: joachim commented#2491687: Make EntityViewsData::getViewsTableForEntityType() public rather than protected has been fixed, so we can use that method.
Comment #2
sumitmadan CreditAttribution: sumitmadan commentedPlease review.
Comment #3
joachim CreditAttribution: joachim commentedThanks!
Your change works, but we should be using EntityViewsData::getViewsTableForEntityType(), now that it's been made public. That has the same logic, and it means that should something change in Views, we're not affected.
Comment #4
BerdirWhat if an entity type doesn't use an entity views data handler? :) The code should at least make sure to not fail then...
Comment #5
sumitmadan CreditAttribution: sumitmadan commentedBy adding
EntityViewsData::getViewsTableForEntityType()gives a warning becausegetViewsTableForEntityType()is not a static method.Also a different issue While adding a flag it displays all the enity types in "Flag Type" field. While enabling it gives an error with following message :
The "contact_form" entity type did not specify a view_builder handler.Does the above issue related to this comment or please suggest how can I check this?
Comment #6
BerdirIt's not meant to be static. You need to call it with $entity_type->getHandler('views_data') and check if it exists with hasHandlerClass()
Comment #7
joachim CreditAttribution: joachim commented> Also a different issue While adding a flag it displays all the enity types in "Flag Type" field. While enabling it gives an error with following message :
The "contact_form" entity type did not specify a view_builder handler.
Can you file a new issue for that please?
Though regarding contact_form entities, see #2409575: config entity types should not be flaggable.
Comment #8
sumitmadan CreditAttribution: sumitmadan commentedI am not so good at object oriented programming. But still I have tried this and I am not sure if I correctly understood it.
Thoughts Please?
Comment #10
Berdir$info->getHandler('views_data')->getViewsTableForEntityType($info);
I'd also recommend to rename $info to $entity_type while touching the code, but I know that @joachim doesn't like even the smallest unrelated changes, so leaving that to him.
Comment #11
sumitmadan CreditAttribution: sumitmadan at QED42 commentedThanks. :)
I have changed the name too.
@joachim Your thought on changing name?
Comment #12
joachim CreditAttribution: joachim commented> but I know that @joachim doesn't like even the smallest unrelated changes, so leaving that to him.
You make me sound like a real nitpicker! :/
Let's go with the name change.
However, true to my reputation:
Needs a space between the if and the (.
If you can fix that, then I think this is good to go :)
Comment #13
sumitmadan CreditAttribution: sumitmadan commentedOops... My Bad!
Comment #14
joachim CreditAttribution: joachim commentedI totally failed to spot this line:
I've applied the patch and removed that before committing.
Thanks everyone :)
Comment #16
sumitmadan CreditAttribution: sumitmadan commentedAgain!!! My Bad!!! Sorry.
Comment #17
joachim CreditAttribution: joachim commentedNo problem! Happens to us all :)
Comment #18
BerdirSorry, didn't mean it in a bad way. I'm just usually a bit more relaxed with stuff like that but I didn't want to derail the issue with something that you'd prefer to happen separately.
Comment #19
joachim CreditAttribution: joachim commentedHa, ok :)
My first co-maintainer and mentor was sun, who tends to be quite strict about things like this (and coding standards too!), and I picked up the same habits :)
Working on a project like Flag, with a really long history, I've really learnt to appreciate the usefulness of keeping commits small and focussed. There have been lots of times when I've had to do git archaeology to try to figure out why a particular bit of code is there and what it's supposed to do, and unfortunately in Flag's history there are quite a few monster commits (including a horrendous 'dump everything from branch A to branch B' commit round about the start of the 6.x-2.x branch, which I always have to step over when using git blame!). So I'm trying to make the lives of future maintainers easier :)