The Vote entity extends ContentEntityBase, but ignores and re-implements many features already provided and implemented in ContentEntityBase. I alluded to this in #3150371: Vote/VoteInterface are documented improperly and incompletely

For example, ContentEntityBase defines base fields id, uuid, language, etc. which are common to ALL entities. Because Vote extends ContentEntityBase, Vote does not need to redefine these base fields, it just needs to properly delegate to the parent class. Because Vote is currently ignoring the parent class, we are duplicating code and losing functionality. Plus this duplicate code makes it harder to maintain - if a bug is fixed or a new feature added to the parent ContentEntityBase, that bug fix / new feature will NOT currently be inherited by Vote.

Here is a work in progress. The point of this patch is to invoke parent::baseFieldDefinitions() within Vote::baseFieldDefinitions so that the Vote entity will use the parent's implementation of the base fields. Right now Vote has a copy of those definitions from the parent rather than just inheriting them.

Note that this patch does not change any existing functionality of the Vote entity, but after the patch the Vote entity will have a few new base fields inherited from ContentEntityBase, so there will need to be a hook_update_N() to update the Vote entity on existing sites. That's why it's still a work in progress - everything but the update hook is complete.

CommentFileSizeAuthor
#17 3153550-17-vote-entity.patch4.11 KBtr
vote-base-fields.patch4.11 KBtr

Issue fork votingapi-3153550

Command icon 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:

About issue forks

Comments

TR created an issue. See original summary.

tr’s picture

tr commented
Related issues: +#2898847: Vote list cache tag not invalidated when saving a vote, +#3153473: Making Votingapi Translatable

Note that #2898847: Vote list cache tag not invalidated when saving a vote is a related issue where again we aren't delegating to the parent, so we're losing functionality - cache tags aren't getting set properly.

Likewise, in #3153473: Making Votingapi Translatable, there is no need to completely redefine base fields etc. just to add language support, as language support will be inherited from the parent ContentEntityBase if we delegate the baseFieldDefinitions() to the parent properly.

tr’s picture

tr commented
Priority: Normal » Major
tr’s picture

tr commented
Related issues: +#2635224: ContentEntityBase should provide field definitions for key fields

Added related issue to link the core issue where ContentEntityBase acquired these base fields.

eelkeblok made their first commit to this issue’s fork.

eelkeblok’s picture

eelkeblok
he/him
Primary language Dutch
Location Netherlands 🇳🇱
commented

Correct me if I'm wrong, but when comparing the old code to that of the ContentEntityBase, I don't think there are actually any database changes needed (for a while there, I though the type column needed to change to bundle, but there actually is a mapping in the annotation). Of course, this will need to be tested - I am currently waiting on a conversion script to turn ~330.000 flags into votes, I guess that should be sufficient - but I am just wondering if I am overlooking something.

tr’s picture

tr commented
Version: 8.x-3.x-dev » 4.0.x-dev
Priority: Major » Critical

tr changed the visibility of the branch 8.x-3.x to hidden.

tr changed the visibility of the branch 3153550-vote-entity-unnecessarily to hidden.

tr changed the visibility of the branch 4.0.x to hidden.

tr opened merge request !30

tr closed merge request !30

tr opened merge request !31

tr closed merge request !31

tr changed the visibility of the branch 3153550-vote-entity to active.

tr changed the visibility of the branch 3153550-vote-entity to hidden.

tr’s picture

tr commented
StatusFileSize
new 3153550-17-vote-entity.patch4.11 KB

I screwed up the issue fork by trying to rebase the changes into 4.0.x. Maybe someone can fix that - I've tried all the simple things and am unwilling to spend more time on it.

So right now I will be relying on the patch instead. Here is a manual re-roll against 4.0.x.

tr changed the visibility of the branch 3153550-vote-entity-unnecessarily to active.

tr’s picture

tr commented
Status: Needs work » Active
guido_s’s picture

guido_s
Location Cologne
commented
Related issues: +#3396329: Stable or beta 4.x D11-compatible release, +#3451390: Automated Drupal 11 compatibility fixes for votingapi, +#3511188: Create a Drupal 11-ready release for Voting API
guido_s’s picture

guido_s
Location Cologne
commented

I just upgraded to 4.0.x-dev Version on a platform where I'm using this module with the Rate module with thumbsup rating and stars in entities (nodes and commerce products), views and webforms and had no issues so far without any database updates.

Upgrading drupal/votingapi (3.0.0-beta5 => dev-4.0.x 010514d)

Would be nice to have a tagged release for security / stability reasons.
When I got @tr right in the D11 issues of this module, this issue here is the reason why there is no tagged D11 release yet.
But I don't see what isn't working and if there really is anything left that needs to be done for this.

guido_s’s picture

guido_s
Location Cologne
commented

Even with the patch from this issue I agree with @eelkeblok that it wouldn't require any db updates.

Also asked the AI:

no update hook needed.

Why:

* The patch only switches to using the parent base field definitions and replaces t() calls with TranslatableMarkup + some label/description text tweaks.
* It does not add or remove any stored fields, indexes, or change any field storage settings.
* The id, uuid, and bundle type fields now come from the parent, but their storage characteristics remain the same (integer unsigned id, UUID readonly, bundle reference to vote_type). The patch just overrides their labels/descriptions.

What to do after applying:

* Just rebuild caches (drush cr). There’s no hook_update_N() or post-update required because no schema changes are introduced.