Vote entity unnecessarily re-implements features provided by base class

Created on 20 June 2020, about 4 years ago

Updated 12 March 2024, 4 months ago

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.

🐛 Bug report

Status

Needs work

Version

3.0

Component

Code

Created by

🇺🇸United States TR Cascadia

Live updates comments and jobs are added and updated live.

Incomplete comments

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

Comment 11 months ago →
🇺🇸United States TR Cascadia
Comment 11 months ago →
🇺🇸United States TR Cascadia
Added related issue to link the core issue where ContentEntityBase acquired these base fields.
First commit to issue fork.
Comment 4 months ago →
🇳🇱Netherlands eelkeblok Netherlands 🇳🇱
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. 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.

contrib.social Blog FAQ Discussions

Production build 0.69.0 2024