Drupal 8 Port of the Scheduler Module

I will start working on the port during the Upgrade your module to Drupal 8 session at drupalcon tomorrow.

Sweet.

I am running a Drupal 8 blog site at the moment, happy to test the port and help with any issues going forward :)

Hi Chris,
Thanks for your interest and offer of help. Yes the D8 port is good news.

One thing that would really help Pieter is if we got the D7 code up to standards so that he has a cleaner base to work from. There are five patches ready for review in #1566024: Coding Standards and Coder Review #17 which clean up nearly all the issues reported by Coder Review

If you could test those patches then we can get them committed to provide a cleaner start for D8, which will make it much easier to keep it that way.

Jonathan

Hi Jonathan,

Will try, currently all my available free-time energy has gone into D8 recently, there again I am quite keen to have a scheduler for Drupal 8 so will see what I can do.

Well I could create a number of separate tasks for updating the module to D8 if people are interested in helping with the port. Like hook_menu() conversions, variables to CMI conversions, ...

I will also document the steps that I will take to convert the module here, this might be useful information for other people that want to do module updates.

Converted the scheduler.info file to scheduler.info.yml

First step is to convert the old scheduler.info file to the new scheduler.info.yml so I can see the module in the module list and enable it.

• Commit: ec86d5d
• Change record: .info files are now .info.yml files

Convert variables to CMI

We have to convert the old variable_get(), variable_set() and variable_del() function calls to the new configuration system (nicknamed "CMI" after the Configuration Management Initiative).

Notes:

• The D7 version of Scheduler uses the "long date format" variable from the System module, but this date format can be deleted in D8 so it cannot be relied on. I opted to make this a configuration option inside Scheduler. This means that this will not be configurable through the interface for the moment. I am also thinking that this should actually be converted to a human readable date when the node is rendered, not when the node is loaded.

   function scheduler_node_load($nodes, $types) {
  +  // The date format in which the (un)publication dates are shown.
  +  // @todo Keep these in unix time, and decide how to display them on rendering.
  +  $date_format = \Drupal::config('scheduler.settings')->get('date_format_long');
      // ...
  -    $row['published'] = $record->publish_on ? date(variable_get('date_format_long', 'l, F j, Y - H:i'), $record->publish_on) : NULL;
  -    $row['unpublished'] = $record->unpublish_on ? date(variable_get('date_format_long', 'l, F j, Y - H:i'), $record->unpublish_on) : NULL;
  +    $row['published'] = $record->publish_on ? date($date_format, $record->publish_on) : NULL;
  +    $row['unpublished'] = $record->unpublish_on ? date($date_format, $record->unpublish_on) : NULL;
  

• Commit: 0eee0d701
• Change record: Added a file-based configuration system
• Documentation: How to upgrade variables into configuration

Up until 1am last night wrestling with views in D8 realistically at this rate I am not going to get around to looking at anything D7 in my spare time out of work for quite some time unfortunately.

Edit: forgot this is not thread, refers to #3

Going to unassign myself from this issue for the time being. I agree with @jonathan1055's suggestion of cleaning up the D7 branch first. I am going to go through the D7 issues first, and restart work on the D8 port when there are no issues left that are marked as "Needs review" or "RTBC".

If someone wants to pick this up for the time being, feel free to do so. There is still a lot of useful work that can be done regardless of the state of the D7 version.

Just so everyone is aware I am working on a d8 port here https://github.com/Examiner/scheduler it is very much a work in progress and ground up rewrite due to the major changes between d7 and d8. Obviously the plan is to get a d8 branch set up on d.o. Right now (waiting on a code review on a pull request)
* Fields are exposed on all node types as base field types using html 5 date elements (need to work on third party settings and next on my list)
* Validation is implemented as per the module now
* Cron is working for publishing and unpublishing with providing node lists etc being exposed via plugins

Todo (on my immediate list)
* Third party settings for per content type config and additional form alter logic
* Implementing permissions
* Default scheduled content view

Other items are somewhat off my radar or are simply ready to be touched eg rules

@marcingy, I see you have not started from the current code base but started from scratch. This means the entire 11 years of history of this project is lost, and it is impossible to backport fixes that are made to the D8 version to the D7 and D6 versions. Do you think it is possible to restart this work using the actual Scheduler git repo and refactoring the existing code rather than rewriting it from scratch?

marcingy, thank you for your interest in this, and for starting to work on it. It's good to have more people helping with the code.

However, I agree with pfrenssen, it would be preferrable to use the existing code-base. I know that there are some major parts which could do with a re-write, but starting from scratch completely divorces from the current project. We'd value your ideas on what should be changed, but I think it needs a bit of discussion here first.

Thanks,
Jonathan

Not saying that it has to be the case here, but sometimes, a fresh start is not a bad idea. Even when starting with the existing code base, many of the relevant API's changed so much in 8.x that I assume almost everything will be different anyway.

A fresh start allows to properly build on D8 entity API patterns for example, with extensible schemas/fields, instead of custom tables and queries.

I am working on the port for my work at examiner.com. I of course have no issue rebasing the code, but one key think is the approach I am using in anyway shape or form acceptable to the scheduler team.

I ask because it really is a rewrite utilising plugins, moving to core base fields and using html5 based dates opposed to string based dates. This was in many ways my initial rational for 'starting from the ground up', as vast amounts of the code is no longer needed if this approach is taken.

I totally agree that the D8 version should use plugins, events, storing our third party settings on the entities, and all the other goodies that are offered by D8, but I think we should approach this through refactoring, not through a rewrite from the ground up.

Even though the module is rather small it has received a staggering 200 bug fixes over the years. This has resulted in a very stable code base in which many edge cases were already identified and fixed. Credit to this, we have had a security release last week, and thousands of sites updated from older (sometimes several years old) versions, and there was not a single complaint in the issue queue.

There is a very nice blog post by Joel Spolsky that explains the pitfalls of doing a rewrite instead of a refactoring: Things you should never do, part I. This post is 15 years old but still very relevant :)

It's tempting to start from scratch and provide a more focused feature set. We are suffering from feature bloat, there are many things that could arguably be removed. But site builders are relying on it the module to be a complete solution to all their scheduling needs. We would like to keep as much of this intact as we can. People rely on even the most obscure features and would like to have an upgrade/migration path for their D7 websites in the future.

We were planning to start on the D8 port very soon. We just finished a sprint last week to fix ALL open issues and release the final (so far :) D7 version. We were planning to do a final cleanup #2170353: Reduce memory footprint and an issue to discover missing test coverage #2112869: 7.x test coverage so we could start the port in earnest. Especially the test coverage would help in ensuring that the D8 port is a success.

I also had some ideas for new features that were not really practical in the D7 code base. I would like to step away from being node-only, but provide a base SchedulerInterface that offers methods publish(), unpublish() and isPublished() - we can implement this for nodes ourselves, but allow people to implement this interface to schedule other entities as well.

Now, I'm pretty excited about the work that has already been done now. It's very inspiring and I want to use as much as possible from it. But due to the nature of commissioned work, I imagine that the Examiner currently has a very real and urgent need to get scheduling of nodes up and running. We cannot move so quickly, and I would not want to impose our slow approach on @marcingy. Also, our more obscure features provide little business value to the Examiner so I expect development to slow down once the basic features are available.

What would be the best way to approach this? Does my reasoning to prefer a refactoring over a rewrite make sense or am I exaggerating? Can we somehow merge our approach the work that was done?

The above makes sense, and basically I am the point where I need to fix a permission issue with the user/%/schedule view and then dev is going to slow down. And to be honest as long as we end up with publish and unpublish being base fields using the html 5 date element then what the API above that ends up exactly looking like is not super important. I really don't want to end up with a fork of the module so more than willing to assist as much as I can.

I'm going to start porting the module. I'll be committing in branch 1982932. I will make sure to make clear commit messages and move one piece at a time so that it is easy to follow. I'm doing this in my spare time so progress will be rather slow.

I welcome all contributions and will be looking at marcingy's port for inspiration.

Thank you for doing it in small bits. I'm following it through, and as you said, this is easier in distinct commits.

I can spend 20-30 minutes per day on it, so the commits are naturally going to be small :)

What shall we do with the functionality that currently exists to tweak the layout of the options in the node form? We have two options now: to choose between displaying the options in a fieldset or vertical tabs, and whether to collapse the fieldset if no scheduled date is set.

In D8 the way the forms are displayed can be configured through the UI. Maybe it's better to let site builders use contrib modules such as Field Group to customize the fields to their liking?

This is great. Thank you.

I'm getting some help from a colleague so I set up a Github repo so that people can contribute more easily: https://github.com/pfrenssen/scheduler. I will accept pull requests there and merge them back to drupal.org.

Let's keep all discussion in here though.

I have "fixed" the 8.x-1.x branch. This contained an earlier attempt at a D8 port from 2013. I have reverted that attempt and brought it up to date with the current 7.x-1.x. I have rebased all the commits from this issue and pushed them so the 8.x-1.x branch.

People that want to contribute, feel free to do either pull requests on the Github clone, or post patches on drupal.org. If you find issues, please file them here and not on Github.

If #2417183: Remove Date Popup integration is applied then for the first time there is something visible in the interface. The module can then be enabled without throwing any errors, and the main configuration form appears at admin/config/content/scheduler. Submitting the form still throws fatal errors though. One step at a time :)

This is great! Thank you for all your work.

I've downloaded the latest 8.x dev from drupal.org but am having difficulty in getting anything to show. Keeps complaining about undefined function module_exists. Anyway, this is not really the thread to discuss my errors, I will keep trying and find out what I've done wrong.

Did you try merging #2417183: Remove Date Popup integration? There were some calls to see if Date Popup was enabled. I'm using the minimal install profile on the latest HEAD of Drupal 8.0.x.

I'll merge #2417183: Remove Date Popup integration and convert the remaining calls to module_exists().

No I did not use that Date Popup patch.

I just upgraded to the latest 8.0.0.beta6 and now I'm getting the usual 'If you have just changed code (for example deployed a new module or moved an existing one)' with the link to https://www.drupal.org/documentation/rebuild

I don't understand if module_exists() does not exist in D8 how come you are not getting these fatal message?

It was probably because in the Date Popup removal patch the calls to module_exists() that caused the fatal error were removed. I've now merged the Date Popup removal patch and updated all other calls to module_exists(). If you have some time can you try again?

Next Thursday I'm going to give an introduction to D8 to some of my colleagues. I will be using Scheduler as a practical example. Will create some issues against 8.x-1.x so that we have something to do.

I'd love to help test Scheduler, but am having more troubles installing/updating to the latest D8 Core. Won't bother you here with the details. Would start a separate thread, but it is not a Scheduler problem.

The development on the D8 port has really taken off since I started making issues. People are doing amazing work, I never expected this response!

Current focus is on replacing all procedural functions that have been removed in Drupal 8 so that we can at least run the code without getting fatal errors about missing function definitions.

Yes it is very exciting to see all this activity when it used to be just you and me on this issue queue. I would like to get involved and try out your work - hope to get some time soon.

Thank you for all the excellent progress.

Jonathan, do you think it would be appropriate to make the protection of the lightweight cron mandatory for D8? When we introduced this for D7 in #2141413: Secure the lightweight cron run page we made this optional and disabled by default to not affect existing implementations. For D8 we could generate a random key when the module is first installed. Do you think this would be worthwhile?

Pieter, yes I agree we should make the cron access key mandatory. Let's continue the discussion on #2626480: scheduler/cron needs a route controller where we have a patch which I am testing.

I have created #2662476: Progress towards 8.x release of Scheduler to help us track what is needed before each release. Currently I would like to hear people's views on that issue regarding what is needed before alpha1. We are nearly there, I'm sure.

Following up the comments in #2662476-52: Progress towards 8.x release of Scheduler I've taken the step to remove the configurable Scheduler format. There is a patch on #2799869-11: Date field placeholder text doesn't reflect date format defined in admin settings - hardcode the format and I would welcome all ideas and feedback.

This a quite a major change, so it has to be done before the next release (rc2) so that we are stable before releasing 8.x-1.0

Scheduler 8.x-1.0 was tagged at 2017-11-14, so let's consider this fixed.