Updated: Comment #1

Problem/Motivation

This is primarily an issue when importing Calendar views. If you export a calendar view the style option groupby_times is not normally exported, unless the database value is overriding the default value defined in the calendar style plugin. When the view is subsequently imported (often via a Feature) the groupby_times option is not initially stored in the database. The existing calendar code attempts to use the database value for groupby_times, which then triggers error messages such as:

For a day view:

Notice: Undefined index: groupby_times in template_preprocess_calendar_day() (line 267 of /foo/sites/all/modules/calendar/theme/theme.inc).
Notice: Undefined index: groupby_times in template_preprocess_calendar_day() (line 271 of /foo/sites/all/modules/calendar/theme/theme.inc).

For a week view:

Notice: Undefined index: groupby_times in template_preprocess_calendar_week() (line 480 of /foo/sites/all/modules/calendar/theme/theme.inc).
Notice: Undefined index: groupby_times in template_preprocess_calendar_week() (line 484 of /foo/sites/all/modules/calendar/theme/theme.inc).

Proposed resolution

There are have been various resolutions proposed to address this issue. For those without the permissions or knowledge to apply a patch you can force the groupby_times option to be stored in the database by editing the view and the day and week display and saving without changing any of the settings (see these instructions). An alternative is to edit the exported view configuration and and manually insert the option. However, the ideal resolution is to fix the code and this was discussed in #1397986: Errors at the day View: Undefined index: groupby_times in template_preprocess_calendar_day()...

Remaining tasks

Review patch below.

User interface changes

None.

API changes

None.

CommentFileSizeAuthor
#18 calendar-2160183-18.patch1.51 KBrudiedirkx
#15 calendar-days-extend-too-far.png7.18 KBbritter
#15 calendar-groupby-times-2160183-2.patch1.4 KBbritter
#1 calendar-groupby-times-2160183-1.patch2.24 KBeft
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

eft’s picture

eft CreditAttribution: eft commented
Issue summary: View changes
Status: Needs work » Needs review
FileSize
calendar-groupby-times-2160183-1.patch2.24 KB

The attached patch attempts to resolve this issue for new and existing sites. Many thanks to FatGuyLaughing, jbylsma, dhalbert and KarenS for their work to date on this issue.

eft’s picture

eft CreditAttribution: eft commented
Issue summary: View changes
PascalAnimateur’s picture

PascalAnimateur CreditAttribution: PascalAnimateur commented

The patch works perfectly on my setup. You get my vote for a commit!

silurius’s picture

silurius CreditAttribution: silurius commented

Edit: Oops, wrong issue, nothing to see here...

guillaumev’s picture

guillaumev CreditAttribution: guillaumev commented
Status: Needs review » Reviewed & tested by the community

Related issue: https://www.drupal.org/node/1895778.

In my case, this patch fixed both the user notice message AND the display of overlapping events in the week and day view... RTBC ?

miromarchi’s picture

miromarchi CreditAttribution: miromarchi commented

+1 for Patch in #1, worked correctly! Thanks

Nemanja’s picture

Nemanja CreditAttribution: Nemanja commented

Patch works for me +1

milovan’s picture

milovan CreditAttribution: milovan commented

Patch works here as well, thanks. Will you include it in next release, please?

ladybug_3777’s picture

ladybug_3777 CreditAttribution: ladybug_3777 commented

Agreed, this patch works! Another vote to include it in the next release please!

dekisha’s picture

dekisha CreditAttribution: dekisha commented

Patch worked for me also +1

michielkenis’s picture

michielkenis CreditAttribution: michielkenis commented

Patch tested and confirmed :)

lias’s picture

lias CreditAttribution: lias commented

This patch works for me as well. Will it be committed?

ademarco’s picture

ademarco CreditAttribution: ademarco at Nuvole commented

Works for me too.

KevinVb’s picture

KevinVb CreditAttribution: KevinVb commented

This patch worked for me too. Please include it in a new release.

britter’s picture

britter CreditAttribution: britter commented
FileSize
calendar-groupby-times-2160183-2.patch1.4 KB
calendar-days-extend-too-far.png7.18 KB

Sorry, I was having issues with multi-day entires extending too far in the view (check the screen shot) so I've devised a similar patch which works, but not as pretty :(

ron_s’s picture

ron_s CreditAttribution: ron_s commented

@eft, one minor issue with patch #1. We should fix the last two modified lines of the patch to have the correct Drupal format. Spaces are missing between the conditional and the bracket.

For example, this...

if ($view->date_info->style_groupby_times_original == 'half'){

...should become this:

if ($view->date_info->style_groupby_times_original == 'half') {
rudiedirkx’s picture

rudiedirkx CreditAttribution: rudiedirkx commented
Status: Reviewed & tested by the community » Needs work

The option value still exists in the plugin handler, so check that. Checking for isset() is weird, because the plugin clearly defines the option. This fix is purely to avoid the Notice, not to fix the problem.

rudiedirkx’s picture

rudiedirkx CreditAttribution: rudiedirkx commented
Status: Needs work » Needs review
FileSize
calendar-2160183-18.patch1.51 KB

This patch uses the plugin's settings, stored in the plugin handler object, where they always exist, because Views adds defaults if they're not in the export.

if ($view->style_plugin->options['groupby_times'] == 'half')

etc

jrearick’s picture

jrearick
he/him
Primary language English
Location Iowa
CreditAttribution: jrearick commented

#18 works for me. Avoiding the isset() if it's always defined in $view->style_plugin->options ['groupby_times'] seems like a good way to go. However, there is still no space before the { in the last two modified if/elsif as mentioned in #16

rudiedirkx’s picture

rudiedirkx CreditAttribution: rudiedirkx commented

The maintainer can add that space if they want, when they add this. Thank you for not Needs work'ing it for a missing space =)

klidifia’s picture

klidifia CreditAttribution: klidifia commented
Status: Needs review » Reviewed & tested by the community

Confirming that #18 works.

So the notices that are appearing actually seem to relate to a bug as well. I haven't looked into it too much but I got 4x these notices when looking at a day that had two events in overlapping times.

I noticed that without the patch applied and the PHP notices showing - that the events were not actually displayed correctly on the page - it was only showing one event and it did not span the correct time.

After applying this patch I can see both events side by side with the correct duration!

Jonathan Young’s picture

Jonathan Young CreditAttribution: Jonathan Young as a volunteer commented

I did a fresh install and ended up with the same issue and #18 solved my problem too

ladybug_3777’s picture

ladybug_3777 CreditAttribution: ladybug_3777 commented

I've removed patch #1 in favor of #18 and it is running smoothly so far!

jomarocas’s picture

jomarocas CreditAttribution: jomarocas as a volunteer commented
Priority: Normal » Major
Status: Reviewed & tested by the community » Patch (to be ported)

confirming that working the patch #18 please post in the next update of calendar

fizk’s picture

fizk CreditAttribution: fizk commented
Status: Patch (to be ported) » Reviewed & tested by the community

Should still be RTBC.

izus’s picture

izus CreditAttribution: izus commented

Will there be a release with this patch in soon please ? we need a release, event alpha or beta one :)

jomarocas’s picture

jomarocas CreditAttribution: jomarocas as a volunteer commented
Status: Reviewed & tested by the community » Patch (to be ported)

Hi tested and for me working good in many projects this is patch to be ported

daften’s picture

daften CreditAttribution: daften at District09 commented
Status: Patch (to be ported) » Reviewed & tested by the community

This should be RTBC @jomarocas, please read the issue status manual: https://www.drupal.org/node/156119 !!!

kobee’s picture

kobee CreditAttribution: kobee as a volunteer commented

OK for me with patch #18. Thanks @rudiedirkx

But one year after the patch, it still not committed to the dev branch?

The calendar module didn't have been updated since 2014 for the 7.x, so apparently it's not maintained anymore..

pcambra’s picture

pcambra
he/him
Primary language Spanish
Location Asturies
CreditAttribution: pcambra commented

+1 to RTBC

  • fizk committed 6164043 on 7.x-3.x authored by rudiedirkx 
    Issue #2160183 by britter, rudiedirkx, eft: Undefined index:...
fizk’s picture

fizk CreditAttribution: fizk commented
Status: Reviewed & tested by the community » Fixed

All good things come to those who wait for far too long. Sorry for the delay :-/

Status: Fixed » Closed (fixed)

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

kingandy’s picture

kingandy CreditAttribution: kingandy commented

Looking forward to a stable release with this in

wylbur’s picture

wylbur CreditAttribution: wylbur commented
Parent issue: » #2568105: Plan for Calendar 7.x-3.6 release
apaderno’s picture

apaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
CreditAttribution: apaderno as a volunteer commented
Assigned: eft » Unassigned
apaderno’s picture

apaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
CreditAttribution: apaderno as a volunteer commented
Title: Undefined index: groupby_times - FIX » Undefined index: groupby_times