Remove dependency of Dynamic Entity Reference (DER)

Created on 19 July 2023, almost 2 years ago

Problem/Motivation

πŸ’¬ using dynamic_entity_reference brings in triggers and uninstalling does not get rid of them Active

Steps to reproduce

Proposed resolution

Clone functionality as suggested by @eiriksm in πŸ› Version 2.0.0-alpha1 is incompatible with Drupal 9.5.4. RTBC

Remaining tasks

User interface changes

API changes

Data model changes

πŸ“Œ Task
Status

Active

Version

2.0

Component

Code

Created by

πŸ‡©πŸ‡ͺGermany c-logemann Frankfurt/M, Germany

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

Merge Requests

Comments & Activities

  • Issue created by @c-logemann
  • πŸ‡©πŸ‡ͺGermany c-logemann Frankfurt/M, Germany
  • First commit to issue fork.
  • Status changed to Needs review over 1 year ago
  • πŸ‡¨πŸ‡¦Canada joelpittet Vancouver

    I'm setting to Needs Review because I did a very rough WIP for an idea on how we could do this.

    The gist of the idea:
    Instead of storing the entity_id field as a DER, just store an internal link and derive the entity data from loading the internal path (kind of like the menu_link_content module in core does)

    That way even if the entity has been deleted we would have a reference to where it came from.

    Does this idea gel with the maintainers?

  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 9.5.x + Environment: PHP 8.1 & MySQL 8
    last update over 1 year ago
    64 pass, 32 fail
  • πŸ‡¨πŸ‡¦Canada joelpittet Vancouver

    The failures are expected BTW, I just need some confirmation on the approach before I tackle those.

  • πŸ‡³πŸ‡΄Norway eiriksm Norway

    I am very open and positive to this. The implementation details can be tweaked too I guess, but I can't see why this would not cover at least most of the usecases. One aspect that I do not know if works or not is if an entity type does not actually have a canonical URL. Is that possible or common? If I created a custom entity type , could I opt to not have it have a canonical URL?

  • πŸ‡©πŸ‡ͺGermany c-logemann Frankfurt/M, Germany

    First of all @joelpittet’s thank you to move forward on this.

    I currently have no time to review because of customer projects. But in one project I will soon create at least one entity type which don't need a canonical route definition. This makes sense when if an entity is just for storing data or its managed via other entities. A good example is the paragraph module and its entity type. Currently there is no "links" section defined.

  • πŸ‡¨πŸ‡¦Canada joelpittet Vancouver

    I guess for the paragraphs module (and others like it, storage β†’ module for example).

    You'd probably want the host entity URL anyway, right? Currently the URL is shown (when possible) on the report page.

  • πŸ‡³πŸ‡΄Norway eiriksm Norway

    Sure but I was referring to being able to load (or save for that matter) the entity you want to check from the URL πŸ€“

  • πŸ‡¨πŸ‡¦Canada joseph.olstad

    joseph.olstad β†’ made their first commit to this issue’s fork.

  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 9.5.x + Environment: PHP 8.1 & MySQL 8
    last update about 1 year ago
    64 pass, 32 fail
  • πŸ‡ΊπŸ‡¦Ukraine quadrexdev Lutsk

    I have started working on this and I hope to finish it within the next few days

  • πŸ‡ΊπŸ‡¦Ukraine quadrexdev Lutsk

    Okay, so I added two base fields: parent_entity_id and parent_entity_type_id instead of using DER field or a link field.

    Adding two base fields for storing entity_type_id and entity_id looks like a simpler and more reliable solution for me.

    I'll spend some more time testing this one. If someone else can test it and/or provide some feedback - feel free :)

  • Status changed to Needs work 10 months ago
  • πŸ‡ΊπŸ‡¦Ukraine abramm Lutsk

    I checked the MR, added a minor suggestion.

  • Status changed to Fixed 10 months ago
  • πŸ‡ΊπŸ‡¦Ukraine quadrexdev Lutsk

    Committed on 2.0.x, thanks everyone for your contribution!

  • πŸ‡©πŸ‡ͺGermany c-logemann Frankfurt/M, Germany

    I didn't found the time to test and code but I want to thank everyone involved to solve this.

  • πŸ‡¨πŸ‡¦Canada joseph.olstad

    Nice work on this! Very good to remove der if it's not needed!

  • Automatically closed - issue fixed for 2 weeks with no activity.

  • πŸ‡§πŸ‡ͺBelgium Frederikvho Belgium

    I just wanted to add that I received a logical error when trying to update this module, because the removed dependency is still installed in the active configuration upon deploy. This causes drush updb to fail. This is normal Drupal deploy behavior and cannot be prevented afaik.

     [error]   (Currently using Missing or invalid module The following module is marked as installed in the core.extension
    configuration, but it is missing:
     * dynamic_entity_reference

    I solved this by doing two deploys:
    First composer require the dependency that was removed in the latest update.
    The rest of the flow can stay the same, so:
    dependency should be removed from core.extension.yml file
    Then deploy.
    Only difference is Drush updb won't fail this time around. Because you will have put the dependency back in place separately.

    For the next deploy you can remove the required dependency, once and for all.

    Both this updb error and my solution seem logic to me, keeping the Drupal deploy strategy in mind, so I didn't see the need to create a new ticket. I just wanted to put this out there in case other developers struggle with.

  • πŸ‡ΊπŸ‡ΈUnited States nmorin Huntsville, AL

    @frederikvho - Thanks for your comment and work-around! I am seeing the same thing running drush updb after upgrade to 2.1.0-alpha1.

  • πŸ‡§πŸ‡ͺBelgium rp7

    Thank you for the efforts made here.

    Just FYI: I'm in the process of upgrading the Linkchecker module on my project and running against some issues related to Views. I have a View listing linkchecker links that also displays (per linkchecker link) a few fields of the linkchecker link's related entity (e.g. the entity the link was made in).

    Turns out the Dynamic Entity Reference module has built-in functionality (dynamic_entity_reference_field_views_data()) that makes this possible. Looks like this functionality is completely lost now.

  • πŸ‡¨πŸ‡¦Canada joseph.olstad

    We've been using linkchecker 2.1.0-alpha1 in production with a few patches, haven't noticed the regression mentioned in #22, not sure I understand what that is.

  • πŸ‡§πŸ‡ͺBelgium rp7

    @jospeh.olstad

    not sure I understand what that is

    For example: you have a link field on content type Article. You enable linkchecker for that particular field. On the linkchecker broken links report (view), you also want to display the article's title besides each checked link. For that, in Views, you'll need a relationship from the linkchecker link entity table to the node table. The ability to add this relation to your view is something that the Dynamic Entity Reference module provided out of the box.

    For anyone looking to restore this functionality, I constructed this piece of code which seems to do the job:

    /**
     * Implements hook_views_data_alter().
     */
    function mymodule_views_data_alter(array &$data) {
      if (!empty($data['linkchecker_link']) && empty($data['linkchecker_link']['node__entity_id']['relationship'])) {
        $entity_type = \Drupal::entityTypeManager()->getDefinition('linkcheckerlink');
        $target_entity_type = \Drupal::entityTypeManager()->getDefinition('node');
    
        $t_args = [
          '@origin_label' => $entity_type->getLabel(),
          '@target_label' => $target_entity_type->getLabel(),
          '@field_name' => 'parent_entity_id',
          '@type' => 'base field',
        ];
    
        $data['linkchecker_link']['node__entity_id']['relationship'] = [
          'title' => new TranslatableMarkup('@field_name to @target_label entities', $t_args),
          'label' => new TranslatableMarkup('@field_name: @target_label', $t_args),
          'group' => $entity_type->getLabel(),
          'help' => new TranslatableMarkup('References to @target_label entities referenced by @field_name @type on @origin_label entities.', $t_args),
          'id' => 'standard',
          'base' => 'node_field_data',
          'entity type' => $target_entity_type->id(),
          'base field' => $target_entity_type->getKey('id'),
          'relationship field' => 'parent_entity_id',
          'extra' => [
            [
              'left_field' => 'parent_entity_type_id',
              'value' => 'node',
            ],
          ],
        ];
      }
    }
    

    Not saying this is something that Linkchecker should offer out of the box (although I think it could come in very handy for some users), just leaving it out here for people running against similar issues.

  • πŸ‡§πŸ‡ͺBelgium rp7

    I found another issue by the way, but this is something that should be fixed in the Dynamic Entity Reference module.

    I upgraded to Linkchecker 2.1.x (with the Dynamic Entity Reference field removed), and was receiving fatal errors about queries trying to write to the entity_id__target_id_int column in the linkcheckerlink table. Appears DER doesn't (always) automatically remove its database triggers. See πŸ“Œ Fix IntColumnHandler::delete Active for more info and a potential fix.

  • πŸ‡©πŸ‡ͺGermany c-logemann Frankfurt/M, Germany

    @rp7
    > Appears DER doesn't (always) automatically remove its database triggers

    Because DER is using database triggers which are more complicate to handle especially in common managed hosting setups is the main reason why we wanted to get rid of it.

    If there is a need for more views integration we can also implement this in linkchecker. Please open a follow up Issue for this and other missing features and merge requests are always welcome.

Production build 0.71.5 2024