Prepend date in page name by hook creates improper results on some existing pages

[sebibu]

Hey Procis, wrote this - and my very first - hook „Prepend date in page name“, yesterday.
Once you get it, this system and the API is magnificent. Thank you @ryan and @all contributers!

On new (news) pages it works great so far. E.g.:
Title: Testnews mit Ümläüten
Altered page name: 2024-09-24-testnews-mit-ümläüten

But on existing pages special characters are removed. E.g.
Title: Partnerschaftliche Unterstützung - Ihr werdet gesucht!
Altered page name: 2023-07-27-fans1991-partnerschaftliche-untersttzung-ihr-werdet-gesucht

Another example is attached.
There you can see, that the ProcessPageEdit-message shows the right path!?

I have this in my site/config.php to allow only german special characters:

$config->pageNameCharset = 'UTF8';
$config->pageNameWhitelist = '-_abcdefghijklmnopqrstuvwxyz0123456789äöüß';

Temporary uninstalled Module „PagePathHistory“ with no look.

The code of my hook (suggestions for improvement are very welcome!):

/* NEWS pages only: prepend date in page-name */
$wire->addHookAfter('Pages::saveReady', function($event) {
  // get current Page object — in this case this is the first argument for the $event object
  $page = $event->arguments(0);
  
  // Only for pages with news template
  if($page->template == 'news') {
    // Test if field day exists, otherwise exit
    if (!$page->template->hasField('day')) {
      $event->message("Field 'day' is missing!");
      return;
    }
    $title = $page->get('title');
	// Sanitize title and substitute special characters
    $optimizedTitle = wire('sanitizer')->pageNameUTF8($title);
    //$optimizedTitle = wire('sanitizer')->pageName($title, Sanitizer::okUTF8); // or translate option

	$date = wireDate('Y-m-d', $page->day);
	// Set output formatting state off, for page manipulation
	$page->of(false);
	$page->name = $date.'-'.$optimizedTitle;
	//$page->save('name');
	$event->message("New saved name is $page->name");
    //$event->message("Path of new saved page is $page->path");
  }
});

Any idea why this doesn't works on all existing news, too?

Thx and 👋
Sebastian

[bernhard]

2 hours ago, sebibu said:

Hey Procis, wrote this - and my very first - hook „Prepend date in page name“, yesterday.
Once you get it, this system and the API is magnificent. Thank you @ryan and @all contributers!

Congrats and welcome to a new world of superpowers 😉 

I don't have details but I'm always using pageNameTranslate:

Not sure why that would work differently on new or existing pages...

2 hours ago, sebibu said:

The code of my hook (suggestions for improvement are very welcome!):

Instead of if(...) { ... } you can use early exit strategy which is usually cleaner in hooks. This is also called "guard clause":

So you could write it like this:

/* NEWS pages only: prepend date in page-name */
$wire->addHookAfter('Pages::saveReady', function($event) {
  // get current Page object — in this case this is the first argument for the $event object
  $page = $event->arguments(0);
  
  // Only for pages with news template
  if($page->template != 'news') return;

  // Test if field day exists, otherwise exit
  if (!$page->template->hasField('day')) {
    $event->message("Field 'day' is missing!");
    return;
  }

  $title = $page->get('title');
  // Sanitize title and substitute special characters
  $optimizedTitle = wire('sanitizer')->pageNameUTF8($title);
  //$optimizedTitle = wire('sanitizer')->pageName($title, Sanitizer::okUTF8); // or translate option

  $date = wireDate('Y-m-d', $page->day);
  // Set output formatting state off, for page manipulation
  $page->of(false);
  $page->name = $date.'-'.$optimizedTitle;
  //$page->save('name');
  $event->message("New saved name is $page->name");
  //$event->message("Path of new saved page is $page->path");
});

The difference is minimal here but using if(...) { ... } is usually harder to read, because you could have an else { } somewhere further down the hook, so when trying to understand your hook you have more workload for your brain. When using if($template != 'whatever') return; you instantly know the hook only applies to 'whatever' templates.

[sebibu]

Thank you @bernhard for helping optimizing the code!👍
For checking the template field I already used a guard clause, but for the template check it makes sense and is more readable to use it, too.

I would like to keep the german special characters like äöüß instead of exchanging (->pageNameTranslate) them with ae, oe, ue, ss.
So wire('sanitizer')->pageNameUTF8($title); should be the way to go. Right?

[bernhard]

Ah, sorry, didn't read carefully enough! Hope it was helpful nonetheless 🙂 

[poljpocket]

On 9/25/2024 at 2:13 PM, bernhard said:

When using if($template != 'whatever') return; you instantly know the hook only applies to 'whatever' templates.

For such cases, even more readability:

$wire->addHookAfter('Pages::saveReady(template=news)', function($event) {
   // [...]
}

This way, the hook doesn't even get called if template isn't news.

Also, I think you can even get rid of the second guard clause like this:

$wire->addHookAfter('Pages::saveReady(template=news,day!=)', function($event) {
   // [...]
}

If the page doesn't have a day field, the selector doesn't match and the hook isn't called. This is untested though 😊

Edited September 27 by poljpocket

[sebibu]

Thank you @poljpocket for this good optimizations!👍

While searching for hooks I stumpled across this syntax but didn't tested and then forgot it.🤓