Problem/Motivation

I wanted to alter the start and end datetime using hook_fullcalendar_process_dates_alter(), but it seems the hooks are not firing. I also checked another hook, hook_fullcalendar_editable(), which also didn't fire when I refresh the calendar page. Is it suppose to fire on the event calendar view page?

Steps to reproduce

1. Define a hook in custom module:

/**
 * Implements hook_fullcalendar_process_dates_alter().
 */
function MYMODULE_fullcalendar_process_dates_alter(&$date1, &$date2, &$context)
{
  $logger = \Drupal::logger('mymodule_logger');
  $logger->info('Fullcalendar process dates alter called. Current dates: @date1, @date2', [
    '@date1' => $date1,
    '@date2' => $date2,
  ]);
  ...
}

2. Enable the module, and refresh cache.
3. Visit the calendar page.
4. Don't see log message in the backend. Alterations didn't apply.

It might be a silly mistake, but I have checked other hooks in the same module file and they work as expected.

Issue fork fullcalendar-3517066

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

hktang created an issue. See original summary.

dcam made their first commit to this issue’s fork.

dcam opened merge request !58

dcam’s picture

dcam commented
Title: Hook not firing » hook_fullcalendar_process_dates_alter is non-functional
Version: 3.0.2 » 3.1.x-dev
Priority: Normal » Major
Status: Active » Needs review

I verified that hook_fullcalendar_process_dates_alter() is non-functional. In the D7 versions this alter hook was invoked in the theme.inc file. It looks like that file was removed and split up during the D8 upgrade. During that process the hook invocation was lost and forgotten.

I restored the invocation, but felt like I had to update its definition. This is a BC break, but BC is broken already and has been for years. I don't think it matters and shouldn't require a new major version. Anyway, the new hook definition works better with FullCalendar::prepareEvent() because I can just plug in existing variables. I think that the context information delivers the same information that it did in D7, just maybe not quite as conveniently, i.e. the field info would have to be extracted from the fields array.

Anyway, I was able to successfully create a hook implementation with the code I've provided in the MR.

I'm setting the priority to Major since this bug renders the hook unusable with no workaround.

dcam changed the visibility of the branch 3517066-process-dates-hook to hidden.

dcam opened merge request !59

dcam’s picture

dcam commented

I hid MR 58 because there was something wrong with it. I think it was caused by me opening the MR while the issue version was set to 3.0.2. MR 59 was properly created from the 3.1.x branch.

mandclu made their first commit to this issue’s fork.

mandclu’s picture

mandclu commented
Status: Needs review » Needs work

As discussed over Slack, let's keep the start and end dates separate in the API instead of passing them within an array.

dcam’s picture

dcam commented
Version: 3.1.x-dev » 3.0.x-dev

dcam closed merge request !59

dcam changed the visibility of the branch 3.0.x to hidden.

dcam opened merge request !66

dcam’s picture

dcam commented
Status: Needs work » Needs review

This ended up being more complicated than I expected. The inherent problem is that the hook which came from D7 passed \DateTime objects. But the modern code mostly deals with datetime strings. It wouldn't be quite as much of a problem if it weren't for the all-day handling. In the end, the simplest thing to do was to change the hook's definition to have the date strings passed instead of objects. We could try to convert everything to use objects instead, but it would force a major refactoring of the code, I think. It makes me wonder if this is how the hook got lost in the first place.

In case you're curious, I think the reason I left the dates in their array was due to a strict reading of the ModuleHandler::alter() docs. It only allows three parameters to be passed to implementations: $data, $context1, and $context2. I'm a very literal person, so I thought "Oh, I should put both dates in the $data variable. And that works out because the entity and fields can be the context variables." But I don't think it really matters.

dcam’s picture

dcam commented

For the record, this updated hook is working for me with the implementation I'd already set up to give end times to events that only had start dates - after updating the implementation of course.

mandclu’s picture

mandclu commented

Thanks for the additional work. This looks great, but I do wonder if we need to pass by reference in the API example. If we can get that cleared up, this should be good to merge.

dcam’s picture

dcam commented

Glad you caught that. Thank you for doing a thorough review of it.

  • mandclu committed 61d17340 on 3.0.x authored by dcam 
    Issue #3517066 by dcam, mandclu: hook_fullcalendar_process_dates_alter...
mandclu’s picture

mandclu commented
Status: Needs review » Fixed

Thanks for your work on this. Merged in.

Status: Fixed » Closed (fixed)

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