Support entity types whose entity bundle type does not end with `_type` and entities without created date

Hi szeidler,
It's great to hear that you have used the new work to implement a plugin for taxonomy terms. Thanks for the detailed feedback. Yes I am not surprised that as we get more plugins, we find more assumptions that need to have options/flexibility now. I found a large number obviously when I first expanded from Node to Media, but because Media was also a core content type there were many things that were the same. Then when we added commerce_product support we found some more that needed fixing. I'm pleased that you have only found these two things (so far)

At first glance your suggestions seem good.

I already had this following change in the pipeline, but it makes sense to add it now, in preparation for testing of other entity types.

Thanks szeidler for adding you plugin files, as that immediately allows me to test the changes.

The check for method_exists($entity, 'getCreatedTime') can be removed, but it also requires the re-ordering of the conditional elements. The option must be checked first, so that if it is negative the rest of the conditional elements will not be checked. As it stood (before my last commit) we still get the error Error: Call to undefined method Drupal\taxonomy\Entity\Term::getCreatedTime() in scheduler_entity_presave(). Testing the option first avoids this.

stumbled over this issue, made a few comments on the MR

Thanks @berdir, those comments are really helpful. Just for background, the 2.x branch is very new and was created specifically so that the huge changes in #2096585: Support for non-node entities, e.g. Media, Commerce Products, Custom 3rd-party entities are started on a new branch, and 8.x-1.x will not have entity plugin support. The only commits to 2.x so far are related to this plugin work.

I would prefer not to add more changes into the above issue, as that MR has been merged with all the main work done. I will close that one soon. Clearly there are improvments that can be made, so I think they could be added to this issue (with the subject made more generic) as that is where @berdir has made the suggestions.

Adding to this thread. Currently also no entities are supported that don't support or need bundles. While going through the code, it seems to be architecturally close to the original node based implementation and relies on the bundles to function. Would it be possible to make it support non bundled entities? If not, at least it needs to be made clear that only bundled entities are supported. An alternative I have discovered is the scheduler field module. Although not too mature, but the approach is more flexible and it incorporates a similar plugin based model to practically have scheduling for tasks beyond publishing/un-publishing.

Any insights on why entity types with no bundles are not supported? Is it actually the case or am I missing something?

Hi rockaholiciam,
Thanks for the prompt. It will take some major investigation but is definitely worth looking at. Can you open a new issue for this please? We will then have the discussion there, and leave this issue for the tasks already in progress.
Jonathan

There is a problem here when Taxonomy module is enabled then Scheduler is installed, because currently the .install process is expecting a view.yml definition. Adding taxonomy into the functional BrowserTestBase (even if not actually testing the new taxonomy plugin) demonstrates the exception.

We have an assumption that a view showing the scheduled items will exist. Without it, the admin does not have any quick and simple way to see which terms are scheduled. They have to view the vocabulary tree and edit each term to see the status and dates. Is this acceptable? The options to solve this are:

1. create a view for scheduled taxonomy terms, and add it as a local task/tab on the taxonomy vocab page, or
2. change the code which expects a view to exist, to make it optional

If we decide on option 1 then we should wait for #3224340: Hardcoded local task dependency on view scheduler_scheduled_content because that has the new functionality for dynamic local tasks.

That's better. Now that we have a scheduled taxonomy view the tests can actually run with the taxonomy module enabled, and we get 8 fails instead of 52. These will be generic failures - we are still not explicitly testing a taxonomy_term entity type yet.

The final test failure is due to a missed occurrence of $entityTypeId . '_type' in SchedulerAdminForm::buildForm(). This can be easily changed to use ->getBundleEntityType() just as has been done in the earlier changes.

NOTE: The tests are not explicitly testing the taxonomy entity type yet, this is just making sure that the existing tests run and pass when the Taxonomy module is enabled.

There is getting to be too much going on in this single issue, so to help clarity (and be more useful as reference for future issues) I have created
#3260067: Implement Taxonomy Term scheduler plugin and
#3260070: Streamline the test coverage process when a new plugin is added

I will revert the changes in this issue back to just covering what is described in the issue title.

Force-pushed the relevent changes to this same branch and MR10.

Merged and fixed. Thanks @szeidler for the original patch and followup fixes.
Thanks also to @berdir for your review. I have not forgotten your comments and will pick them up on a different issue, as they are not directly relating to the changes here.

The followup issues #3260067: Implement Taxonomy Term scheduler plugin and #3260070: Streamline the test coverage process when a new plugin is added will have the remainder of the work I did here.