Add support for media entities

Nice start, but there are things that needs to be changed if we don't want to drown in duplicated code and dependency issues with existing implementations.

Some point that needs work:

1. Make a submodule of this code, in that way you can set a dependency for Media and won't break existing site config.
2. Rename: scheduler_update_8003 to scheduler_update_8002
3. Update scheduler.schema.yml met Media types (check Media in core for exact names)
4. Set dependency for Core Media in .info.yml file
5. PHPDOC @see in _scheduler_media_bundle_edit_form_alter should be referencing scheduler_form_media_type_edit_form_alter()
6. Create interface for SchedulerManager and implement both classes with that interface.
7. Create base class 'SchedulerManagerBase' and extend one for node and one for media, set SchedulerManager as deprecated en extend SchedulerNodeManager
8. .. stopped reviewing, will try to get some time to help with this issue.

I did some of the sugestions in #9.
Thanks

Small tip on links within translatable strings:

$this->t('Change your <a href=":url">Settings</a>', [':url' => Url::fromRoute('route.of.url')->toString()]);

This can also be used for the logger messages.

I was thinking that would be better to split this feature request in two parts (open another feature request or task).

The first, to prepare the scheduler to be prepared to integrate with other entities: This part would have the Manager base, the interface and the changes necessary in the scheduler module.

The second (this feature request), only to integrate with media.

What do you think?

Update patch to work with Scheduler version 8.x-1.1.

I applied patch #15 on scheduler 1.x-dev. When I run system cron, I get a php error: TypeError: Argument 1 passed to Drupal\scheduler\SchedulerManagerBase::isAllowed() must be an instance of Drupal\Core\Entity\Entity, instance of Drupal\media\Entity\Media given, called in /web/modules/contrib/scheduler/scheduler_media_integration/src/SchedulerMediaIntegrationManager.php on line 103 in Drupal\scheduler\SchedulerManagerBase->isAllowed() (line 104 of /web/modules/contrib/scheduler/src/SchedulerManagerBase.php)

When I run Lightweight cron from admin/config/content/scheduler/cron I don't get that php error, but the set expiration date of my media entities has no effect either.

Edit: Also newly created Media entities scheduled to be published in the future will get published immediately on saving.

This is now functional, but still needs work.

Patch #17 uses 9 deprecated calls, some of which will be removed in D9:

drupal_set_message in scheduler_media_integration/scheduler_media_integration.module:145

 * @deprecated in drupal:8.5.0 and is removed from drupal:9.0.0.
 *   Use \Drupal\Core\Messenger\MessengerInterface::addMessage() instead.

entity_get_bundles in scheduler_media_integration/scheduler_media_integration.module:236

 * @deprecated in drupal:8.0.0 and is removed from drupal:9.0.0. Use
 *   \Drupal\Core\Entity\EntityTypeBundleInfoInterface::getBundleInfo() for a
 *   single bundle, or
 *   \Drupal\Core\Entity\EntityTypeBundleInfoInterface::getAllBundleInfo() for
 *   all bundles.

$this->entityTypeManager->getFieldDefinitions in src/SchedulerManager.php:102 & src/SchedulerManager.php:282

   * @deprecated in drupal:8.0.0 and is removed from drupal:9.0.0.
   *   Use \Drupal\Core\Entity\EntityFieldManagerInterface::getFieldDefinitions()
   *   instead.

$entity->link in scheduler_media_integration/src/SchedulerMediaIntegrationManager.php:144 & scheduler_media_integration/src/SchedulerMediaIntegrationManager.php:298

   * @deprecated in drupal:8.0.0 and is removed from drupal:9.0.0.
   *   Use \Drupal\Core\EntityInterface::toLink()->toString() instead.

REQUEST_TIME in scheduler_media_integration/src/SchedulerMediaIntegrationManager.php:134 & scheduler_media_integration/src/Plugin/Validation/Constraint/SchedulerMediaIntegrationPublishOnConstraintValidator.php:23 & scheduler_media_integration/src/Plugin/Validation/Constraint/SchedulerMediaIntegrationUnpublishOnConstraintValidator.php:45

 * @deprecated in drupal:8.3.0 and is removed from drupal:10.0.0.
 *   Use \Drupal::time()->getRequestTime();

Further minor nitpicks:

In scheduler_media_integration/scheduler_media_integration.module:231 replaced * @return \Drupal\node\EntityTypeInterface[] with * @return \Drupal\Core\Entity\EntityTypeInterface[]

How can apply this patch in Drupal 9?

Hi,

usage of composer patches is documented on github: https://github.com/cweagans/composer-patches#usage

Might be also helpful: https://stackoverflow.com/questions/55683488/apply-drupal-8-patch-by-com...

This patch in particular would be:

        "patches": {
            "drupal/scheduler": {
                "Add support for media entities": "https://www.drupal.org/......patch"
            }
        }

Hi maosmurf
Thank you

I can confirm patch #17 and also patch #18 apply on scheduler 8.x-1.3 (but not on the latest dev version).

Everything works as expected. Only there is an error warning in the logs on publishing:
Warning: Creating default object from empty value in Drupal\scheduler_media_integration\SchedulerMediaIntegrationManager->publish() (line 160 of /path/to/my/d8/web/modules/contrib/scheduler/scheduler_media_integration/src/SchedulerMediaIntegrationManager.php)

I have seen that the installation file scheduler_media_integration.install contains code to create tables in the database, which is commented out. A comment was left there: "Create fields manually". In my use case I did not create fields manually, but the submodule works anyway. So what's the point of this? And where is the data stored in the database?

Edit: Ok, found out it is stored in media_field_data. So I think the above mentioned installation file is no longer needed.

I have created a new patch that rolls up 17/18 into one that should install correctly on 8.x-1.3-dev. I had to override a couple of classes in scheduler_media_integration (EventScheduler, EventBase) to remove the depency on Node objects to make everything work.

As there is a patch, changing to NR to get automatic testing.

Also unassigned dysproseum (who did great work at the start, but has not been back for two and half years) as it is better show the current state of assignment/non-assignment. Many others have since contributed.

A couple of minor things with this patch - on the Media type's "edit" page, in the "Scheduler" section:

1. "Node edit page layout" should be "Media entity edit page layout" (or something similar).
2. Inside the "Node edit page layout" fieldset, neither the "Display scheduling options as" nor "Expand fieldset or vertical tab" radio buttons have any effect on media add/edit pages.

-mike

Hi cobblestone.consulting and ultimike, and everyone else who has contributed,

A couple of minor things with this patch

Unfortunately, it is much more than a couple of minor things. The patch as it stands is extremely far away for being considered for committing.

1. The biggest problem is that the new sub-module "scheduler_media_integration" is not an integration at all, but almost a complete duplication of the main Scheduler processing. The patch modifies only three existing scheduler files, and creates 15 new files which are virtually direct copies of existing files. This duplication is absolutely impossible to maintain, is bad programming practice and is totally unviable as a solution. For example all of the work between #17 and #18 to remove deprecated code would not have been necessary as this was already fixed in the main scheduler files. Having duplicated code is not the way to proceed.
2. There is no test coverage for the new functionality
3. The submodule's files are all placed in the base folder, whereas they should be in a dedicated sub-folder. For an example, look at the scheduler_rules_integration sub-module
4. There are 69 coding standards faults
5. The patch contains three unwanted files (2 x .orig and 1 x .rej) that are left-over artifacts from failed patching

Points 2 - 5 are listed here for info, however the fundamental design needs to be addressed in point 1 first. @Tessa Bakker had some great suggestions in comment #9 and I like the idea of a SchedulerManagerInterface and a base class SchedulerManagerBase with implementations for Node, Media (and others ...) and the work done on this in these patches is going to be useful.

There is discussion about this on #2096585-6: Support for non-node entities, e.g. Media, Commerce Products, Custom 3rd-party entities I'm not trying to be negative, or block this work, but it needs to be done in a future-maintainable way, and a generic solution without too much duplication is the correct way forward.

Thanks for the feedback, jonathan1055. My only goal originally was to get patches 17/18 to apply to the dev branch. Clearly I should have paid closer attention to what was in the patches.

I am willing to take a closer look and spend the time to make this more maintainable. I've been looking through it and working on a proof of concept for the last week or so. I decided to create a new scheduler plugin type to manage different entity types. This will eliminate the duplication of code and should make it much easier to support additional entity types.

I'm at a point where I thought it would be a good idea to run this by you before going too much further. The plugins provide the basic info about the entity type - the name itself, the list of forms that need to be altered (entity, entity type), the list of bundles, etc. but it will obviously require quite a bit of change to the module itself.

Are you ok with this approach?

Thanks.

Hi cobblestone.consulting,
Yes that sounds like an excellent approach and I'd be happy to see what you have got so far, and to help with further work. Have you got a GitHub account? If so, you can fork from https://github.com/jonathan1055/scheduler and create a Pull Request which we can share and work on.

Great, thanks! I'll get a fork started.

@jonathan1055 - fork is in place at https://github.com/mgcobblestone/scheduler

I pushed an initial commit up to the dev branch and added a comment in the discussion area describing where I'm at (early stages :-) ).

@cobblestone.consulting has started work on this, and we have a new branch https://github.com/jonathan1055/scheduler/tree/2096585-entity-plugin

I am closing this issue as a duplicate, in favour of #2096585: Support for non-node entities, e.g. Media, Commerce Products, Custom 3rd-party entities which is earlier, and is the more generic solution. Conversation will continue over on that issue. Anyone here, please follow that issue and help test the new code. It is early days at the moment, but looks promising. Thanks all.

Appreciate the work here and in https://www.drupal.org/project/scheduler/issues/2096585

Also, I fully agree that duplicating a dozen of files is not a maintainable solution.

that being said, I needed to use scheduler_media_integration from #18 in drupal9, which requires the usual core_version_requirement: ^8 || ^9

Hi maosmurf,
We've been busy in the last few days and actually have a working version of the entity plugins implementation on scheduler pull request 10. It would be very good if you (or anyone else here) could try that out, and give us any feedback. It is fully operational for Node and Media entities and works at 8.x and 9.x just the same as the rest of the Scheduler functionality. You can get a concise patch for all the latest work via scheduler/pull/10.diff or if you want to see all the separate commits use scheduler/pull/10.patch Both of these ultimately have the same changes so you can take your pick.

I was about to re-open this issue anyway, as being open gives more people the chance to find it and test the new changes.

Here is a patch for the fully functional entity plugins for Scheduler, with implementations for media and node entity types. Thanks to @cobblestone.consulting. This is the same patch as #12 on #2096585-12: Support for non-node entities, e.g. Media, Commerce Products, Custom 3rd-party entities however there are 21 followers on this issue (and 18 on that one) so I don't want to close either issue just yet.

@dysproseum, @Tessa Bakker, @felribeiro, @brunapimenta, @kreatIL, @henokmikre, @sharma.sachin013, @ultimike - you all wanted this so please, if possible, test the patch and give us feedback. There are still some things to do, but it is working properly and will allow you to evaluate this new feature. Thanks.

Thank you @cobbleston.consulting and @jonathan1055 for your efforts on this.

I have applied the patch and submitted it for testing by my client.

thanks,
-mike

I would also like to thank you and promise to help soon by testing.

Thanks @ultimike and @kreatIL. We've done some more work since the patch above on 9th Jan. That one should be fully operational, so if you are using it, the testing will be valid.

The new features since then, and added in to patch #37, are:
(a) support for devel_generate to add scheduling dates to automatically generated media
(b) new permission to for scheduling media separately from scheduling node content
(c) support for Tokens to display the scheduled dates of media items.

The one big thing missing so far is the equivalent of the 'scheduled content' view for admins to see a list of media that is scheduled. I am working on that now.

OK so Devel_generate 2.0 does not have the support for media items. Scheduler's composer criteria for testing is "drupal/devel_generate": "^2.0 || 4.x-dev" which only loaded 2.0. But now that Devel has a fully-released 4.0 branch we can require specifically that - "drupal/devel_generate": "^4.0"

Warning - no NOT use patch #32 as it fails for 2 reasons:

1) SchedulerMediaIntegrationManager calls $entity->link, which is undefined for Media
2) SchedulerMediaIntegrationManager calls $entity->setPublished(FALSE) which was deprecated (providing FALSE as argument) and from D9 onwards leads to actually publishing the entity!

On a side note, I could not apply patch #39 via composer:

  - Installing drupal/scheduler (1.3.0): Extracting archive
  - Applying patches for drupal/scheduler
    https://www.drupal.org/files/issues/2021-01-14/2981703-39.support-non-node-entities.patch (Add support for media entities)
   Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2021-01-14/2981703-39.support-non-node-entities.patch


  [Exception]
  Cannot apply patch Add support for media entities (https://www.drupal.org/files/issues/2021-01-14/2981703-39.support-non-node-entities.patch)!

Fixed #40

Fixed #41. Still, this whole issue is not really D9 ready.

Looking forward to test out work in https://www.drupal.org/project/scheduler/issues/2096585 !

@maosmurf , thanks for your interest, but please will you stop posting patches here which are not going to be committed, ever. You are confusing the workflow.

You also mentioned that you tried to install patch #39 (which is the "real" patch) via composer but it failed to apply. Your error message does not give a lot of information but I suspect it is because you had Installing drupal/scheduler (1.3.0) which will be the 8.x-1.3 tagged release. The patch is designed to be applied to the latest -dev code, as per the recognised standard practice.

It is great to support media entities, but why hardcoding the support of media like it was already hardcoded nodes ?

Content Moderation module do something like that to "guess" which entity types to support:

    foreach ($entity_types as $entity_type_id => $entity_type) {
      // Internal entity types should never be moderated, and the 'path_alias'
      // entity type needs to be excluded for now.
      // @todo Enable moderation for path aliases after they become publishable
      //   in https://www.drupal.org/project/drupal/issues/3007669.
      if ($entity_type->isRevisionable() && !$entity_type->isInternal() && $entity_type_id !== 'path_alias') {
        $entity_types[$entity_type_id] = $this->addModerationToEntityType($entity_type);
      }
    }

Source: Drupal\content_moderation\EntityTypeInfo

With Drupal Core only entities, that's mean:

• Custom blocks
• Nodes
• Medias

And the, it adds a specific base field to those entity types to handle the expected data, like what i done on scheduler_entity_base_field_info().

It would be more future-proof doing something like that, isn't it ?

Hi pdureau,
Before I reply in detail, are you looking at the patch in #34 and #39?
Jonathan

@pdureau as noted in above, these patches are NOT recommended anymore, for several reasons (code duplication, not D9 ready, ...)
see https://www.drupal.org/project/scheduler/issues/2096585

Sorry @jonathan1055 for causing confusion here

Thanks maosmurf, no apology needed.

I have added an updated patch over on #2096585-13: Support for non-node entities, e.g. Media, Commerce Products, Custom 3rd-party entities

The work on #2096585: Support for non-node entities, e.g. Media, Commerce Products, Custom 3rd-party entities is now ready for final testing. Please take a look, test, and let me know how you get on ... over on that issue, not here :-)

Duplicate of #2096585: Support for non-node entities, e.g. Media, Commerce Products, Custom 3rd-party entities.

Thank you nedjo but I was specifically leaving this issue open to keep it on peoples dashboards and issue lists. There are 23 followers here, so having the issue open means that some of them may test the MR/patch on the 'real' issue and give some feedback.

#2096585: Support for non-node entities, e.g. Media, Commerce Products, Custom 3rd-party entities is now committed and fixed, and I have released Scheduler 2.0.0-alpha1

See #3249325: Plan for Scheduler 8.x-1.5 release for the issues currently being worked on, in preparation for the full 2.0.0. release.

Thanks to everyone here for your contributions.

Just the rerolling the patch to 8.x-1.4 version using the #39.

Hey, i'm rerolling this patch to make it possible to use in 8.x-1.5

The last patch had an error, updating it.

Hi sophiavs,
Please don't keep posting to this closed issue. Media support will never be committed to 8.x-1.x, so if you want that feature it would be better to upgrade to Scheduler 2.0 which is now released.