I've been working on a module which uses this hook. However, it doesn't add items but replaces them. Shouldn't $items = $feed be $items[] = $feed?

  // Merge in items from other sources.
  foreach (module_implements('calendar_add_items') as $module) {
    $function = $module .'_calendar_add_items';
    if (function_exists($function)) {
      if ($feeds = $function($view)) {
        foreach ($feeds as $feed) {
          $items = $feed;
        }
      }
    }
  }
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

finfin82’s picture

im working on it too, today. (the stable version of calendar)

i guess you are right with your assumption. ;-)

so agree

btw: is the usage of calendar_add_item anywhere explained? i was debuggin all objects working together with calendar to figure out how to use it ;-)

shushu’s picture

Hi,

I have a calendar view in which I want to fill every empty (no events) cell with something.
I don't think playing with preprocess is the right place to do things in this case, so I think calendar_add_item is exactly what I need.

But like you, I see no-one uses this hook.

Did you use it ?

About the bug, there is any - when it is being called it is done like this:
$nodes = array_merge($nodes, (array) calendar_add_items($view, 'calendar'));

shushu’s picture

Status: Active » Closed (fixed)
Kars-T’s picture

Title: Use of calendar_add_items » Use of hook_calendar_add_items
Category: bug » feature
Status: Closed (fixed) » Needs review
FileSize
1.93 KB

@shushu You are using Drupal 5 but this is about hook_calendar_add_items from Drupal 6.

On the issue:
Using this hook is a pain because you have to build a calendar conform object.

My best guess what fields are needed from debugging this thing is:

/*
 * Implementation of hook_calendar_add_items().
 */
function cp_birthday_calendar_add_items($view) {
  $date = date_make_date('now');

  $node = new stdClass();
  $node->node_title = '123456';
  $node->calendar_start_date = $date;
  $node->calendar_end_date = $date;
  $nodes[][] = $node;

  return $nodes;
}

This "node" will show in the calendar.
You have to use exact Views field names!
If you have no matching name no data will be shown.

But still I can only replace the items in the calendar not really work with them. I made a patch that let's us work on the items by reference and do what we want to do. Now we can call:

/*
 * Implementation of hook_calendar_add_items().
 */
function cp_birthday_calendar_add_items($view, &$items) {
  $date = date_make_date('now');

  $node = new stdClass();
  $node->node_title = '123456';
  $node->calendar_start_date = $date;
  $node->calendar_end_date = $date;
  $items[] = $node;
}
Gribnif’s picture

Personally, my take on the code is that there's a typo. This line:

$items = $feed;

should probably be:

$items += $feed;
Kars-T’s picture

@Gribnif You mean in the original code?

The problem is that you can't change anything given here. This is basically an alteration hook so you can work on the given data which otherwise would be impossible. So += can add items but not change them and only fixes a part of the problem.

An implementaion of drupal alter would be an alternative. But this can only give us one item by reference. The $view object might be needed as it contains data we can use.

Gribnif’s picture

@Kars-T: What I'm trying to say is that the original name of the hook is "add_items", which implies that all it was intended to do is add. As written, the code replaces instead of adding, so the change I proposed makes the code do what it was intended to do.

IMHO, if you want to be able to modify the items, you should create a patch to add an "alter" hook. You should not try to shoehorn this into a hook that was intended for another purpose.

Kars-T’s picture

@Gribnif I see your point but I wouldn't add more hooks than necessary. So not imho ;)

Hope a maintainer comes around and shares some opinion what (s)he would commit to the module.

I promise I make an new patch asap may it be += and or an alteration hook! :)

cangeceiro’s picture

FileSize
631 bytes

Ran into the same issue with this hook. Attached is a patch that does not break ical's use of the hook, but allows you to add items to the calendar as well with the following syntax

  $date = date_make_date('now');
  $node = new stdClass();
  $node->node_title = 'blah';
  $node->calendar_start_date = $date;
  $node->calendar_end_date = $date;
  $nodes[] = $node;

the patch just simply replaces the code with the following

  foreach (module_implements('calendar_add_items') as $module) {
    $function = $module .'_calendar_add_items';
    if (function_exists($function)) {
      if ($feeds = $function($view)) {
        $items = array_merge($items, $feeds);
      }
    }
  }
Kars-T’s picture

@cangeceiro

You patch is the same thing Gribnif suggested but with a different syntax. += for an array equals array_merge().

My patch from #4 if superoir to this method as it lets you alter the array of objects and not only add things.

I still hope a Maintainer would come around and share hers / his thoughts if adding or alteration is the way to go :(

miro_dietiker’s picture

I don't know any other hook that adds items.
There might be hooks like hook_menu to announce things like interfaces and functionality.

However - regarding data and objects - alteration is the use case.
So instead of discussing about how to add, append or replace data and how to name it, simply follow the drupal standard:

Add a hook to directly alter the items - like Kars-T requested.
Since the hook is currently almost unused i suggest renaming it as soon as possible to have a clean name like hook_calendar_items_alter($view, &$items)

Kars-T’s picture

Title: Use of hook_calendar_add_items » Add hook_calendar_items_alter to allow us changing the calendar items
FileSize
1.06 KB

In regards of miros suggestion I changed the hook to a drupal_alter which makes more sense. Please test this patch if you now can easily change any data that is displayed inside of the calendar.

mahnster’s picture

Issue tags: +hooks, +alter, +calendar, +add

Can I ask why the hook_calendar_add_items() hook was removed for the above patch?? Should we also just remove hook_menu() from Drupal and just use hook_menu_alter()??

That doesn't make sense. A hook_alter should be to alter an info array that other modules ALREADY implanted, where as a hook_add() function would be making it clear that we are just adding our own modules info array. This doesn't make sense to be picking one or the other for the calendar module. So many other Drupal features, from form hooks to menu hooks have multiple hooks.

I agree that $items = $feed should be changed to $items[] = $feed, but why get rid of this wonderful API feature? Adding is different than altering!!!! Right?

Another way to look at this, and the important need of hook_add and hook_alter is what if your module puts some items into the calendar, but it is called Zebra, and my module is called Elephant? My module CAN'T alter your items if you add them through hook_alter.

No, the proper way is that all modules FIRST get to ADD their items (by weight, and then module name in alphabetical order if weight is the same) and THEN for each module get to ALTER the existing items, not to add more (not good form if you add items here, but should have a good reason to do so in hook_alter if you do so)

Does that make sense? We need to not destroy the power of Drupal and Drupal hooks. Please make this have both hook_add and hook_alter. If this patch is implemented into Calendar's next release, it will break about 5 of my modules if there is no hook_calendar_add_items().

mahnster’s picture

Seriously, how many of you have modules that add their menu items in hook_menu_alter()?

mahnster’s picture

Also, in the original calendar module code, why isn't all that code for hook_calendar_add_items() just:

<?php
$items += module_invoke_all('calendar_add_items', $view);
?>

We need to stop re-inventing the wheel people, haha. That is exactly what module_invoke_all() does for us.

Kars-T’s picture

FileSize
2.73 KB

Hi richmahn.

I think your idea is good and I altered the patch to add both. An hook to add and an alter to change. So any module can add items and you still can change them after that.

But please cool off that aggresive and overdone polemic. I mean 3 posts, no patch file and many "???". Stuff like "Should we also just remove hook_menu() from Drupal and just use hook_menu_alter()?? ".

We should try and work here together.

$items += module_invoke_all('calendar_add_items', $view);

This can't be used as "+= " is a union of the arrays and the left one would win in the merge.
http://www.php.net/manual/en/language.operators.array.php

I used the same hook name for ical and normal calendar so you can provide a global handling.

Kars-T’s picture

Paths where wrong. Uploading a new file.

skruf’s picture

Thanks for the patch for altering items. Shouldn't the second argument be the items rather than the view as we want to alter the items and the second argument is the only one that gets passed by reference? Like this:

  // Alter the calendar items.
  drupal_alter('calendar_items', $items, $view);
Neslee Canil Pinto’s picture

Issue summary: View changes
Status: Needs review » Closed (outdated)
Issue tags: -

The D6 branch is no longer supported so we're closing this issue. If you think this issue still exists in D7 you can open a new one.