Add revision support

Created on 24 July 2023, over 1 year ago

Problem/Motivation

Creating this issue for version 2.X based off 8.x-1.x-dev. https://www.drupal.org/project/eck/issues/2788507 Add revision support Needs review . Copying the text from this issue to here.

For a lot of use cases you use ECK in a way which is similar to paragraphs module. Once you have also any kind of workflow on those sites, you need revision support for the referenced entities, using entity_reference_revisions.

Given that it would be quite helpful to have revision support as part of ECK

Proposed resolution
Make it possible to switch of revision support when creating an entity type
Default to TRUE
Backport the changes in #2721313: Upgrade path between revisionable / non-revisionable entities in some temporary subclasses inside ECK
Provide an update path for all entity types
Write tests for it

Feature request
Status

Active

Version

2.0

Component

Code

Created by

🇺🇸United States chambers

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @chambers
  • Status changed to Needs review over 1 year ago
  • It works for new ECK types created after patching.

    -----------------------
    Test for types created before the patching.

    Drupal\Core\Entity\Query\QueryException: No revision table for eck_2_type, invalid query. in Drupal\Core\Entity\Query\Sql\Query->prepare() (line 99 of /var/www/html/local6site/web/core/lib/Drupal/Core/Entity/Query/Sql/Query.php).
    

    Testing with workflow

    If your existing eck type has content moderation workflow;

    Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'mydatabase.dr_' doesn't exist: SELECT "base"."id" AS "id", "base"."type" AS "type", "base"."uuid" AS "uuid", "base"."langcode" AS "langcode" FROM "dr_eck_1_type" "base" INNER JOIN "dr_" "revision" ON "revision"."id" = "base"."id" AND "revision"."" IN (:revisionIds__0); Array ( [:revisionIds__0] => ) in Drupal\Core\Entity\Sql\SqlContentEntityStorage->doLoadMultipleRevisionsFieldItems() (line 624 of /var/www/html/local6site/web/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).
    

    Tested with drupal 10.1.2 & PHP 8.1

  • Status changed to Closed: duplicate over 1 year ago
  • 🇨🇦Canada RobLoach Earth

    There's been a lot of work on this over at https://www.drupal.org/project/eck/issues/2788507 Add revision support Needs review . Mind if we combine them?

  • 🇺🇸United States chambers

    This one shouldn't have been closed as duplicate for 2 reasons.
    1. The other issue is for version 8.x-1.x-dev. This one is for version 2.0.0
    2. The other issue patches didn't work for this version because of Drupal 10 changes and this is for version 2.0.0.

    So this is unrelated to the other issue.

    Sorry, I never got an email there were responses.

  • Status changed to Needs review over 1 year ago
  • 🇨🇦Canada RobLoach Earth

    Oops! Good catch, thanks.

  • First commit to issue fork.
  • 🇧🇪Belgium dieterholvoet Brussels

    Created a MR based on the patch, will review there.

  • Merge request !43Add revision support → (Open) created by dieterholvoet
  • Status changed to Needs work about 1 year ago
  • 🇧🇪Belgium dieterholvoet Brussels

    I left some comments in the MR. Additionally, a generic revision UI was added in Drupal 10.1 ( Implement a generic revision UI Fixed ), so we should probably rewrite some stuff to make use of that.

  • 🇧🇪Belgium dieterholvoet Brussels

    The changes from #2788507-116 Add revision support Needs review should still be added here as well.

  • 🇺🇸United States hungdo

    Attaching a patch file that works for eck 2.0.0 on my local.

  • Status changed to Needs review 5 months ago
  • Hello,

    I have created a patch based on changes available on #2788507-116 Add revision support Needs review for the 2.0.0.

    Also the changes from #2788507-116 Add revision support Needs review do not work for existing eck entities. so I have modified the update_hook which will make the existing entities revisionable.

    Note - This patch will make all the existing eck entities revisionable.

  • 🇺🇸United States hungdo

    In our project, not all our custom entity types requires the revision enabled, however the patch #15 force all entity types revisionable, we can't enable turn it off, so I updated the patch to handle the revisionable entity types only.

  • 🇬🇧United Kingdom T-lo Bristol, UK
  • 🇧🇪Belgium dieterholvoet Brussels

    Crediting the people who worked on Add revision support Needs review .

  • 🇧🇪Belgium dieterholvoet Brussels

    Please don't post new patches, work is being done in the MR. My comments there haven't been addressed yet, so setting back to Needs work.

  • 🇧🇪Belgium dieterholvoet Brussels

    I added support for the core revision UI based on #116 in Add revision support Needs review . Since we'll need to bump the minimum core version 10.1 to make use of the core revision UI, I propose we only merge this once a stable minor release has come out with all the previously merged issues.

  • I have encountered this error before, but I think the problem is related to MR!43.

    Tested with Drupal 10.4.1 ECK 2.1.0-beta1 & 2.x-dev .
    The error seems to be a bit serious. After these attempts, the test site cannot return to its previous state, sql:drop is required. A fresh test site was used each time.Since I could not pass this stage, I did not have the opportunity to test it with other related eck modules.

    Here's the dblog records with fresh install.

    When I try to add the entity type, if I don't select the revisionable option;

    Drupal\Core\Entity\Exception\UnsupportedEntityTypeDefinitionException: The entity type organization does not have an "revision_created" entity revision metadata key. in Drupal\eck\Entity\EckEntity::revisionLogBaseFieldDefinitions() (line 35 of core/lib/Drupal/Core/Entity/RevisionLogEntityTrait.php).
    

    If I select the revisionable option;

    Drupal\Core\Entity\Exception\UnsupportedEntityTypeDefinitionException: The entity type organization does not have an "owner" entity key. in Drupal\eck\Entity\EckEntity::ownerBaseFieldDefinitions() (line 33 of core/modules/user/src/EntityOwnerTrait.php).
    

    Detailed log

    The website encountered an unexpected error. Try again later.
    
    Drupal\Core\Entity\Exception\UnsupportedEntityTypeDefinitionException: The entity type organization does not have an "owner" entity key. in Drupal\eck\Entity\EckEntity::ownerBaseFieldDefinitions() (line 33 of core/modules/user/src/EntityOwnerTrait.php).
    Drupal\eck\Entity\EckEntity::baseFieldDefinitions() (Line: 230)
    Drupal\Core\Entity\EntityFieldManager->buildBaseFieldDefinitions() (Line: 195)
    Drupal\Core\Entity\EntityFieldManager->getBaseFieldDefinitions() (Line: 454)
    Drupal\Core\Entity\EntityFieldManager->getFieldStorageDefinitions() (Line: 490)
    Drupal\Core\Entity\EntityFieldManager->getActiveFieldStorageDefinitions() (Line: 187)
    Drupal\Core\Entity\Sql\SqlContentEntityStorage->__construct() (Line: 149)
    Drupal\Core\Entity\Sql\SqlContentEntityStorage::createInstance() (Line: 280)
    Drupal\Core\Entity\EntityTypeManager->createHandlerInstance() (Line: 84)
    Drupal\trash\TrashEntityTypeManager->createHandlerInstance() (Line: 68)
    Drupal\trash\TrashEntityTypeManager->getHandler() (Line: 51)
    Drupal\trash\TrashEntityTypeManager->getStorage() (Line: 69)
    Drupal\Core\Entity\EntityTypeListener->onEntityTypeCreate() (Line: 150)
    Drupal\eck\EntityUpdateService->doEntityUpdate() (Line: 120)
    Drupal\eck\EntityUpdateService->applyUpdates() (Line: 144)
    Drupal\eck\Entity\EckEntityType->postSave() (Line: 563)
    Drupal\Core\Entity\EntityStorageBase->doPostSave() (Line: 489)
    Drupal\Core\Entity\EntityStorageBase->save() (Line: 257)
    Drupal\Core\Config\Entity\ConfigEntityStorage->save() (Line: 354)
    Drupal\Core\Entity\EntityBase->save() (Line: 617)
    Drupal\Core\Config\Entity\ConfigEntityBase->save() (Line: 194)
    Drupal\eck\Form\EntityType\EckEntityTypeFormBase->save()
    call_user_func_array() (Line: 129)
    Drupal\Core\Form\FormSubmitter->executeSubmitHandlers() (Line: 67)
    Drupal\Core\Form\FormSubmitter->doSubmitForm() (Line: 597)
    Drupal\Core\Form\FormBuilder->processForm() (Line: 326)
    Drupal\Core\Form\FormBuilder->buildForm() (Line: 73)
    Drupal\Core\Controller\FormController->getContentResult()
    call_user_func_array() (Line: 123)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 638)
    Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 121)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (Line: 97)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 181)
    Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 76)
    Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 53)
    Drupal\Core\StackMiddleware\Session->handle() (Line: 48)
    Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 28)
    Drupal\Core\StackMiddleware\ContentLength->handle() (Line: 32)
    Drupal\big_pipe\StackMiddleware\ContentLength->handle() (Line: 116)
    Drupal\page_cache\StackMiddleware\PageCache->pass() (Line: 90)
    Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 57)
    Drupal\advban\AdvbanMiddleware->handle() (Line: 50)
    Drupal\ban\BanMiddleware->handle() (Line: 48)
    Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51)
    Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 36)
    Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 51)
    Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 741)
    Drupal\Core\DrupalKernel->handle() (Line: 19)
    
  • Revision should be optional rather than mandatory, as each field of each entity type is independent, and adding a revision field would create many tables in database for those who do not need revision.

    Just a suggestion, no argument.

  • Status changed to Needs work 1 day ago
  • 🇺🇸United States jienckebd

    #32 Pearls -- I ran in to same problem. Workaround:

    In eck_entity_type_build(), replace:

    $definition = array_merge($definition, $base_definition);
    

    with:

    $definition = array_merge_recursive($definition, $base_definition);
    
Production build 0.71.5 2024