Excluding a past event from a PageReference field after it's been chosen

[a-ok]

Hi folks,

I have a PageReference field which is selecting events (that are in date by a date field) and articles with a specific template. My hook in order to allow those pages is as follows:

$wire->addHookAfter('InputfieldPage::getSelectablePages', function($event) {

	if ($event->object->hasField == 'home_whatson') {

		// Get in date events
		$events = new PageArray();
		foreach ($event->pages->find("template=events-detail, events_detail_dates.events_detail_dates_start_date>=today, sort=events_detail_dates.events_detail_dates_start_date, sort=name") as $e) {
			$events->add($e);
		}

		// Get Our Picks articles
		$articles = new PageArray();
		foreach ($event->pages->find("template=our-picks-detail, sort=sort") as $article) {
			$articles->add($article);
		}

		// Push events into the pages we allowed
		$events->import($articles);

		$event->return = $events;

	}

}

This works well but if an event has passed (before it is removed from the PageReference field), obviously it doesn't 'remove' it from the list it simply hides it (even though it's still technically 'selected'). I then need to do a check on the front end for this as well so it isn't returning the items that have passed.

I thought this would work:

$today = strtotime("now"); $articles = $page->home_whatson->filter("events_detail_dates.events_detail_dates_start_date>$today");

But it's obviously now ignoring the articles with the specific template as the filter is removing it from the list (as this should only be applied to events).

Is there a way round this? Should I put removing it from the original query (using ->remove()) or is there some other way?

Any help is appreciated.

[Robin S]

As a general issue...

It's a tricky one, how pages that are already selected in a Page Reference field should be handled if they no longer meet the requirements for selectable pages for that field.

When getting the value of the Page Reference field via the API, I think the value should be validated through the isValidPage() method when it is "woken up" from the database. Currently the field value is validated on sleep but not on wakeup. Validating on wakeup would solve the situation where the value of the field when accessed from the API is different to the apparent value when viewing the field in Page Edit. But wouldn't solve the fact that the database value still includes an invalid page and so could give unwanted matches in $pages->find() for example.

Or if that is not desirable for some reason then I think there should be some improvement to how invalid items are handled in the inputfield. Currently they are hidden in the inputfield, but that means that if you just view the page in Page Edit but do not save then you get a false impression of what the value of the field is. Maybe if invalid items were shown in the inputfield but highlighted in a warning colour, and then those items removed when the page is saved.

Would be good to hear what others think about how invalid selected pages should be handled.

 

For your specific issue...

If you are foreaching the pages in your template, you could check the template and date of each item to determine if it should be output or not.

Edit: you could actually use FieldtypePage::isValidPage() to validate each page in the Page Reference field value in your template, e.g.

/* @var FieldtypePage $field */
$field = $fields->home_whatson;
foreach($page->home_whatson as $item) {
    if($modules->FieldtypePage->isValidPage($item, $field, $page)) {
        // output $item
    }
}

 

[a-ok]

Thanks for your reply, Robin. This makes a lot of sense and I like your breakdown of this a lot.

I don't know too much about the isValidPage() method... can't find too much on the docs about it. Your code snippet line

if($modules->FieldtypePage->isValidPage($item, $field, $page)) {

What is this doing? isValidPage might be what I'm after but slightly confused by the arguments...

Edit: Ah I see (I missed the $field declaration) but unfortunately this is still returning the event that has passed (even though it's now hidden from the PageField).

[a-ok]

I can check each item in the loop... just always strive to resolve everything in the query but maybe not possible in this case.

[Robin S]

33 minutes ago, oma said:

I don't know too much about the isValidPage() method... can't find too much on the docs about it.

It is a public method of the FieldtypePage class. See the description of the arguments in the DocBlock.

35 minutes ago, oma said:

but unfortunately this is still returning the event that has passed

Not sure. It works for me. But if you're not going to be changing the conditions for selectable pages often then you can just check the template and date in the foreach instead.

30 minutes ago, oma said:

just always strive to resolve everything in the query but maybe not possible in this case

There is no performance reason to not use a foreach. It's not like a $pages->find() selector where the database is involved. When you do anything with the value of a Page Reference field the entire PageArray (aka WireArray) is loaded (more about that here) and methods like find() or filter() are just foreaching the WireArray internally anyway.

[a-ok]

This is all really great, Robin. Thanks.

That's super interesting to know about the use of foreach loops... for some reason I would've thought it was using a lot of extra power to check the loop with a foreach but sounds like it doesn't. That's all great.

One final point... if I am adding my dates within a repeater (to allow for multiple dates) – sometimes there's one date row and sometimes there will be more, per event. Obviously what I want to do is only return the event if at least one of the dates in greater or equal to today. Is it possible to do this within the loop? Would I use a break, of sorts when one is 'in date'? On some of the pages I use this page selector

$pages->find("template=events-detail, events_detail_dates.events_detail_dates_start_date>=today, sort=events_detail_dates.events_detail_dates_start_date, sort=name");

I'm assuming this query is doing what I want, right? Or is it only looking at the first repeater row (events_detail_dates) or checking all of them?

EDIT: I believe the selector above is checking all the repeater rows (just checked it) but the sort option is still sorting by the date that is passed (whereas it should be sorting by the in date date... if you get me).

[a-ok]

I came up with this for the loop check...

<?php $today = strtotime("now"); $articles = $page->home_whatson; ?>
<?php foreach ($articles as $article) : ?>
	<?php if ($article->template->name == 'events-detail') : // Event ?>
		<?php foreach ($article->events_detail_dates as $date) : // Loop through the dates ?>
			<?php if ($date->events_detail_dates_start_date >= $today) : // If event hasn't passed then continue ?>
				<?php include('./inc/events-item.inc'); break; // At least one date is upcoming so break after this as it's already been proved successful ?>
			<?php endif; ?>
		<?php endforeach; ?>
	<?php else : // Our Pick ?>
		<?php include('./inc/pick-item.inc'); ?>
	<?php endif; ?>
<?php endforeach; ?>

 

[Robin S]

6 hours ago, oma said:

for some reason I would've thought it was using a lot of extra power to check the loop with a foreach but sounds like it doesn't

Of course it depends on what you are iterating over, but in general foreach is very efficient. For instance, I've read that when working with a plain PHP array it is often more efficient to use foreach in place of dedicated PHP functions such as array_map and array_reduce (although for most situations that would be micro-optimisation and you would just use whichever is most convenient).

 

6 hours ago, oma said:

I believe the selector above is checking all the repeater rows (just checked it) but the sort option is still sorting by the date that is passed (whereas it should be sorting by the in date date... if you get me).

I don't think that kind of sort would be possible in a selector, where you want to look through all repeater items and sort their containing pages by the most distant date. I think when you sort by a value contained within a multiple-page field (repeater, page reference, etc) the sort happens on whatever is the first page in that multiple-page field. So therefore you would need to apply some automatic sort to the repeater items so that the most distant date is always first, and that isn't so easy to do. If you have Profields Table then that has an option to automatically sort by a column. Or you might have to create a hidden field in the event template and populate it with a saveReady hook so it contains the most distant date, then sort by that field.

 

4 hours ago, oma said:

I came up with this for the loop check...

<?php $today = strtotime("now"); $articles = $page->home_whatson; ?>
<?php foreach ($articles as $article) : ?>
	<?php if ($article->template->name == 'events-detail') : // Event ?>
		<?php foreach ($article->events_detail_dates as $date) : // Loop through the dates ?>
			<?php if ($date->events_detail_dates_start_date >= $today) : // If event hasn't passed then continue ?>
				<?php include('./inc/events-item.inc'); break; // At least one date is upcoming so break after this as it's already been proved successful ?>
			<?php endif; ?>
		<?php endforeach; ?>
	<?php else : // Our Pick ?>
		<?php include('./inc/pick-item.inc'); ?>
	<?php endif; ?>
<?php endforeach; ?>

 

To shorten your code you could do away with the nested foreach and do something like:

if($article->template->name == 'events-detail') {
    if(count($article->events_detail_dates->find("events_detail_dates_start_date >= $today"))) include './inc/events-item.inc';
} //...

BTW, where you are not mixing HTML and PHP you don't need PHP open/close tags on every line - you can just put the whole block of PHP code between a single pair of open/close tags. It doesn't do any harm the way you are doing it, but perhaps a little harder to read.

[a-ok]

18 hours ago, Robin S said:

To shorten your code you could do away with the nested foreach and do something like:

if($article->template->name == 'events-detail') {
    if(count($article->events_detail_dates->find("events_detail_dates_start_date >= $today"))) include './inc/events-item.inc';
} //...

Thanks for this, Robin. I wasn't actually aware you could loop through the repeater using find... some some reason. You were dead right re mixing HTML/PHP. Not sure why I wasn't doing that. I used your example but it spat out an error Exception: Unknown Selector operator: '' -- was your selector value properly escaped? field='events_detail_dates_start_date', value='>= 1511108093', selector: 'events_detail_dates_start_date >= 1511108093' (in /Users/rich/Sites/Sites/ofp/wire/core/Selectors.php line 378) but I've ran with this for now...

foreach ($articles as $article) {

	if ($article->template->name == 'events-detail') {

		foreach ($article->events_detail_dates as $date) { // Loop through the dates
			if ($date->events_detail_dates_start_date >= $today) include('./inc/events-item.inc'); break;
		}

	} else {
		include('./inc/pick-item.inc');
	}
	if ($count == $random) {
		include("./inc/cta.inc");
	}

	$count++;

}

 

18 hours ago, Robin S said:

I don't think that kind of sort would be possible in a selector, where you want to look through all repeater items and sort their containing pages by the most distant date. I think when you sort by a value contained within a multiple-page field (repeater, page reference, etc) the sort happens on whatever is the first page in that multiple-page field. So therefore you would need to apply some automatic sort to the repeater items so that the most distant date is always first, and that isn't so easy to do. If you have Profields Table then that has an option to automatically sort by a column. Or you might have to create a hidden field in the event template and populate it with a saveReady hook so it contains the most distant date, then sort by that field.

 

I like this idea a lot. I think what I would try to do is write a saveReady hook that grabs the date that's not been passed yet, add it to a hidden field, and sort by that. That would work for both one date and 2+ dates, right? As long as I can do my check that would work.. and be fairly robust.

[a-ok]

This is what I ended up with for the hook...

$wire->addHookAfter('Pages::saveReady', function($event) {
	$page = $event->arguments(0);
	if ($page->template->name == 'events-detail') { // What's On detail
		$today = strtotime("now");
		$dateRepeater = $page->events_detail_dates;
		foreach ($dateRepeater as $row) {
			$row->of(false);
			if ($row->events_detail_dates_start_date >= $today) {
				$page->set("events_detail_dates_sort_date", $row->events_detail_dates_start_date);
                $page->save("events_detail_dates_sort_date");
				break;
			}
		}
	}
});

EDIT: Only issue is that it won't auto update... only when the page is saved whereas obviously it would need to update when the date has passed so maybe it needs to check it every day? Is that mad? Or like you said "automatic sort to the repeater items so that the most distant date is always first".

I was thinking of using lazyCron (although the $event arguments only return the seconds...)

function myHook(HookEvent $event) {

	$page = $event->arguments(0);
	if ($page->template->name == 'events-detail') { // What's On detail

		$page->message("TEST!");

		$today = strtotime("now");
		$dateRepeater = $page->events_detail_dates;
		foreach ($dateRepeater as $row) {
			$row->of(false);
			if ($row->events_detail_dates_start_date >= $today) {
				$page->set("events_detail_dates_sort_date", $row->events_detail_dates_start_date);
                $page->save("events_detail_dates_sort_date");
				break;
			}
		}
	}

}
wire()->addHook('LazyCron::every30Seconds', null, 'myHook');
wire()->addHookAfter('Pages::saveReady', null, 'myHook');

 

[Robin S]

3 hours ago, oma said:

I used your example but it spat out an error Exception: Unknown Selector operator

Oh right, that's because you can't use spaces around field/operator/value in selector strings. I was just composing that in the browser. If you change to...

if($article->template->name == 'events-detail') {
    if(count($article->events_detail_dates->find("events_detail_dates_start_date>=$today"))) include './inc/events-item.inc';
} //...

...it should work.

 

3 hours ago, oma said:

Only issue is that it won't auto update... only when the page is saved whereas obviously it would need to update when the date has passed so maybe it needs to check it every day?

Not sure why it would need to auto update apart from when the page is saved. It only needs to update if you are making a change to the page by adding or removing a date. In terms of old dates you exclude those in the $pages->find() selector that gets your results:

events_detail_dates_sort_date>=$today

 

[a-ok]

10 minutes ago, Robin S said:

Not sure why it would need to auto update apart from when the page is saved. It only needs to update if you are making a change to the page by adding or removing a date. In terms of old dates you exclude those in the $pages->find() selector that gets your results:

Ah right this is part of the hook, sorry, to sort the output (when returning all events). So if I was to go down the route of having a hidden field... that was populated by the latest date that hasn't passed so I could sort by that value then if I had three sets of dates.. on save it would set the hidden field's value to the date that hasn't passed yet but obviously once that date has passed it would need to update it to the next date that hasn't passed (or don't if it doesn't exist).

You see it sets the sort date as the second date (as that hasn't passed yet) but when it has it should update the sort date field to the next date but alas this only happens on page save, right? So it wouldn't know to auto update it.

$pages->find("template=events-detail, events_detail_dates_sort_date>=today, sort=events_detail_dates_sort_date, sort=name");

function myHook(HookEvent $event) {

	$page = $event->arguments(0);
	if ($page->template->name == 'events-detail') { // What's On detail

		$today = strtotime("now");
		$dateRepeater = $page->events_detail_dates;
		foreach ($dateRepeater as $row) {
			$row->of(false);
			if ($row->events_detail_dates_start_date >= $today) {
				$page->set("events_detail_dates_sort_date", $row->events_detail_dates_start_date);
                $page->save("events_detail_dates_sort_date");
				break;
			}
		}
	}

}
wire()->addHookAfter('Pages::saveReady', null, 'myHook');

 

[Robin S]

9 hours ago, oma said:

it should update the sort date field to the next date

I misunderstood - I thought you wanted to sort by the most distant upcoming date, not the soonest upcoming date. So you'll need a different approach I think, where you loop over your result pages add them to a new array using the next upcoming date as the key, then you sort the array by key. Edit: after thinking some more, this is not an ideal solution because your upcoming date timestamps may not be unique, so they don't make good candidates to use as array keys. A better idea is to add the next upcoming date timestamp as a custom property of each result.

So the general principles...

1. Keep the "events_detail_dates_sort_date" field in your template, but instead of populating it with the soonest upcoming date you populate with the most distant upcoming date in the saveReady hook. This field will not be used to sort the pages (so you could consider renaming it if you like) but rather to exclude pages that have no upcoming date in your $pages->find() selector, so that you are not having to process more pages than necessary.

2. Loop over the raw unsorted results of your $pages->find() query, adding the timestamp of the next upcoming date as a custom property. Then sort on that property:

// For each of your $pages->find() results...
foreach($results as $result) {
    // Test for the template of the result here
    // ...
    // Get the repeater page with the next upcoming date
    $next_upcoming = $result->events_detail_dates->get("events_detail_dates_start_date>=$today, sort=events_detail_dates_start_date");
    // Add the timestamp of the next upcoming date to the result as a custom property
    $result->next_upcoming = $next_upcoming->getUnformatted('events_detail_dates_start_date');
}
// Sort the results by the custom property
$results->sort('next_upcoming');
// Now loop over $results to output the markup

You'll need to expand on this to deal with the other template ("Our pick") you are including in your results. So you would first test for the template of the result page and use the above if it is "events-detail" and come up with some other numerical value for the "our pick" results to use as "next_upcoming" so they appear where you want them when sorted by the custom property.

 

Edited November 20, 2017 by Robin S
Changed to a better solution using a custom page property

[a-ok]

14 hours ago, Robin S said:

$next_upcoming = $result->events_detail_dates->get("events_detail_dates_start_date>=$today, sort=events_detail_dates_start_date");

Hi Robin,

Yes! This all makes sense  I have one issue... the line quoted above at my end is returning NULL when dumping out $next_upcoming. The dates in the CMS are in the future... do you think 'get' is the wrong method? EDIT: I hadn't set up $today yet (as was using simply today in the find selector but obviously need to define $today for the get selector).

I think this is it working! Could be pretty beneficial to people this... hopefully!

Thank you!

P.S Also had no idea you could add custom properties like that

[a-ok]

There does seem to be one issue when the $next_upcoming date is the same date as today ($today = strtotime("now");) it seems to set it as NULL (setting a date before or after today is fine... just when it is today (which'll obviously inevitable happen).

So, the event that is today... this is being included in the original query (obviously)

// Set up the original query (to find all events in date)
$today = strtotime("now"); $events = $pages->find("template=events-detail, events_detail_dates_sort_date>=today"); 

But when we go into the foreach and set the $next_upcoming... even though it should be returning it's returning NULL... could it be that the original today (PW's today) is different from $today (set by string to time now)? I changed them both to $today and now there's no errors but it doesn't include the event (even though it should as it should be >= $today).

To get this to work I had to set $today to a UNIX timestamp of the date (rather than using time/now).

$dateFormat = date('D. j F Y'); $today = strtotime($dateFormat);

 

[DaveP]

Just kinda thinking out loud, but I'm not sure 

$today = strtotime("now");

is going to work as expected. See http://php.net/manual/en/datetime.formats.php

[a-ok]

22 minutes ago, DaveP said:

Just kinda thinking out loud, but I'm not sure 

$today = strtotime("now");

is going to work as expected. See http://php.net/manual/en/datetime.formats.php

Thanks for help, Dave.

I originally had this but for some reason the date set in the CMS (to today's date) has passed when using strtotime("now") my guess is that the date today starts at midnight whereas now is... well, now so in a sense it has passed (in UNIX time).

[Robin S]

@oma, when you use a date/time as a string in a $pages->find() selector it is internally passed through strtotime(). So if you are doing a separate strtotime() elsewhere that you want to give the same result you must use the same string, i.e. use "today" for both or "now" for both.