Expand Publication Date to all Entity Types with Settings Form

Created on 17 June 2021, over 3 years ago
Updated 18 June 2024, 5 months ago

Problem/Motivation

I have custom entity types on my site where I would like to apply Publication Date. The current Publication Date module only applies to Node entities.

Proposed resolution

By introducing a settings form that presents all content entity types, and having the Publication Date module read the settings form when creating tokens, views handlers, and base field info the Publication Date features can be expanded to all content entity types on a given site.

User interface changes

This patch introduces a settings form at Admin > Config > Development > Publication Date Settings where entity types can be selected.

Data model changes

When an entity type is selected on the Settings Form, it is written to the config file publication_date.settings. On submit, a method is executed that creates a post_update hook for the entity type to perform the schema changes on that entity type. Site managers must run the updates via drush or update.php. If an entity is unchecked, the update hook is deleted. Once an update hook has been run, the type can be unchecked, but this will only make the Publication Date inaccessible through the admin, it won't delete the column or the data.

I've refactored all of the places in the code where 'node' was used explicitly and replaced them with function calls that read in the configuration and enable the code for selected entity types.

✨ Feature request
Status

Needs work

Version

2.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States drewble

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Comments & Activities

Not all content is available!

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

  • πŸ‡ΊπŸ‡ΈUnited States awm

    updated the patch to use filesystemInterface to avoid this error
    TypeError: Drupal\publication_date\PublicationDateUpdate::__construct(): Argument #3 ($fileSystem) must be of type Drupal\Core\File\FileSystem, Drupal\s3fs\S3fsFileService given

  • πŸ‡ΊπŸ‡ΈUnited States awm

    correct patch file.

  • πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

    Thank you for the patch. However, creating PHP files on the fly and run them from temp dir it's a terrible idea. Don't do that, it's a security issue.

    I am thinking to an approach where the list of entity types is not stored in config but in code, e.g., by exposing a hook whose invocation returns a list of entity types. Then 3rd-party code implements that hook tell which entity types they like to receive the field. Publication date will implement that hook and only return ['node']. An update hook, will iterate over all entity types and check if the field is installed. If not, will do the proper storage definition install.

  • Status changed to Needs work 6 months ago
  • πŸ‡΅πŸ‡ΉPortugal dxvargas

    I agree with @claudiu.cristea that creating files on the fly is not a good idea.

    Also, I don't see the reason to do this. Can you please explain?
    Why not to do the storage updates immediately when saving the config?
    I've done this experience and it worked. I can submit a patch.

    About the config page, I don't see a problem with that.
    Using hooks would also work but I actually prefer using the configuration page.

    I'll reopen the issue for the discussion to continue and the patch to be improved.

  • πŸ‡ΊπŸ‡ΈUnited States awm

    > creating PHP files on the fly and run them from temp dir it's a terrible idea. Don't do that, it's a security issue.

    Agreed...

  • πŸ‡΅πŸ‡ΉPortugal dxvargas

    I'm uploading a patch with the approach given by @claudiu.cristea, with a hook whose invocation returns a list of entity types that should have the publication date.
    It's far from being a final patch but it works in a basic way and may be useful for next steps.

  • πŸ‡­πŸ‡ΊHungary huzooka Hungary πŸ‡­πŸ‡ΊπŸ‡ͺπŸ‡Ί
    1. +++ b/publication_date.module
      @@ -52,17 +52,21 @@ function publication_date_entity_base_field_info(EntityTypeInterface $entity_typ
      +function publication_date_form_alter(&$form, FormStateInterface $form_state, $form_id) {
      +  $type = isset($form_state->getBuildInfo()['base_form_id']) ? preg_split("/_form$/", $form_state->getBuildInfo()['base_form_id'])[0] : '';
      +  if (!\Drupal::service('publication_date.enabled_entity_types')->isEnabled($type)) {
      +    return;
      +  }
         $account = \Drupal::currentUser();
      -  $node = $form_state->getFormObject()->getEntity();
      +  $entity = $form_state->getFormObject()->getEntity();
       ...
      }
      

      For first, I would check whether the form object is a ContentEntityFormInterface. If so, then we can be sure that the FormstateInterface->getFormObject()->getEntity() returns a content entity – then we can have the entity type ID without any assumption made on the base form ID:

      function publication_date_form_alter(&$form, FormStateInterface $form_state): void {
        $form_object = $form_state->getFormObject();
        if (!$form_object instanceof EntityFormInterface) {
          return;
        }
        $entity = $form_object->getEntity();
        if (!$entity instanceof FieldableEntityInterface) {
          return;
        }
        $entity_type_id = $entity->getEntityTypeId();
        if (!\Drupal::service('publication_date.enabled_entity_types')->isEnabled($entity_type_id)) {
          return;
        }
       ...
      }
      
    2. +++ b/publication_date.module
      @@ -52,17 +52,21 @@ function publication_date_entity_base_field_info(EntityTypeInterface $entity_typ
      -    $form['published_at']['#access'] = $account->hasPermission('set any published on date') || $account->hasPermission('set ' . $node->bundle() . ' published on date');
      +    $form['published_at']['#access'] = $account->hasPermission('set any published on date') || $account->hasPermission('set ' . $entity->bundle() . ' published on date');
      

      These permissions need to be adjusted: e.g. both node and taxonomy_term entities might have a "foo" bundle.

    3. +++ b/publication_date.services.yml
      @@ -1,5 +1,10 @@
      +    class: Drupal\publication_date\PublicationDateEnabledTypes
      +    arguments:
      +      - '@module_handler'
         publication_date.event_subscriber:
           class: Drupal\publication_date\EventSubscriber\PublicationDateSubscriber
      +    arguments: ['@publication_date.enabled_entity_types']
           tags:
      

      Can we standardize the format of the arguments?

      Personally, I prefer the format currently used on " publication_date.enabled_entity_types" since it is git-friendly.

    4. +++ b/publication_date.tokens.inc
      @@ -13,11 +13,15 @@ use Drupal\Core\Render\BubbleableMetadata;
      +  sleep(0);
      

      I guess this is only a debug leftover.

    5. +++ b/publication_date.tokens.inc
      @@ -35,22 +39,22 @@ function publication_date_tokens($type, $tokens, array $data, array $options, Bu
           /** @var \Drupal\node\NodeInterface $node */
      -    $node = $data['node'];
      +    $entity = $data[$type];
       
      

      This variable comment does not make any sense...

      Maybe it can be replaced by a condition:

      $entity = $data['type'];
      if (!$entity instanceof FieldableEntityInterface) {
        return;
      }
      
    6. +++ b/src/PublicationDateEnabledTypes.php
      @@ -0,0 +1,62 @@
      +
      +
      +namespace Drupal\publication_date;
      +
      +
      +use Drupal\Core\Extension\ModuleHandlerInterface;
      

      Nit: Unnecessary empty lines.

    7. +++ b/src/PublicationDateEnabledTypes.php
      @@ -0,0 +1,62 @@
      + * Class PublicationDateEnabledTypes
      + * @package Drupal\publication_date
      

      Niz: Leftover from IDE default s, needs a proper class description.

    8. +++ b/src/PublicationDateEnabledTypes.php
      @@ -0,0 +1,62 @@
      +  }
      +}
      

      Nit: missing empty line.

    Obviously, we still need some magic to install / uninstall the field whenever it is needed, which (if possible) could be resolved by a ConfigEvent subscriber (save, delete).

  • πŸ‡΅πŸ‡ΉPortugal dxvargas

    I'm now submitting a new patch with some changes suggested by @huzooka in the previous comment.

    1. Done
    2. To do
    3. Done
    4. Removed
    5. Comment removed. I got into a situation where the entity was not an instance of FieldableEntityInterface, for that reason I'm not using your suggestion.
    6. Done
    7. Done
    8. Done
Production build 0.71.5 2024