[4.x] Update child URIs when parent slug changes

[jacksleight]

Fixes #9445

This PR implements a new listener that updates the URI index for all child pages whenever the parent page is updated.

It should probably only run if the slug has actually changed, but I'm not sure if there's a way to check that in an EntrySaved listener?

I couldn't figure out the right way or place to do the test, if you can point me in the right direction I'll add that in.

[duncanmcclean]

I wonder if it might be better for this logic to live in the Entry@save method instead so it's next to the rest of the save logic?

Also, with it being a listener on the EntrySaved event, it won't trigger if someone's doing $entry->saveQuietly().

[jacksleight]

Oh yeah good point. I only did it like that because I was copying the other UpdateStructuredEntryUris listener that runs for tree updates, but saveQuietly is definitely an issue.

Shall I change it?

[jasonvarga]

I think its okay where it is 👌

It should probably only run if the slug has actually changed, but I'm not sure if there's a way to check that in an EntrySaved listener?

There's no way at the moment, but should be once #5502 gets implemented.

[jasonvarga]

Since #5502 is pretty close to being merged, I'll mark this as a draft until we implement only doing this when the slug changes.

[jasonvarga]

Once the isDirty PR is ready, use it to make sure that all of this only happens if the slug has changed.

[jacksleight]

Cool, will do.

[jacksleight]

Once the isDirty PR is ready, use it to make sure that all of this only happens if the slug has changed.

Done

[jasonvarga]

Duncan was right. Not sure why I was okay with leaving it as an event. Anyway, I updated it for you.

Thanks both!

[nadinengland]

@jasonvarga this change is causing me an issue which results in an infinite loop.

The Stache driver appears to sidestep this (haven't looked too much into it as to why) but I'm using a custom Eloquent driver and it's triggering a infinite loop for me. Completely appreciate you can't support anyone's custom stuff, but that isn't the root cause of my issue as far as I can tell.

This is because empty() php method returns false for objects: empty(collect()) === false in the updateChildPageUris() method.

I believe this should be changed to use Laravel's blank() which checks to see if it's a Laravel Collection.

Separately, this is a bit difficult to patch in the short term as the method is private, can't be overridden. It might have been discussed at length before, so apologies if this is rehashing that, but switching this and many other method to protected helps to make the code more flexible when issues like this crop up as we can monkey patch them in the container :)

Thanks as always for all your great work.

[duncanmcclean]

The Stache driver appears to sidestep this (haven't looked too much into it as to why) but I'm using a custom Eloquent driver and it's triggering an infinite loop for me. Completely appreciate you can't support anyone's custom stuff, but that isn't the root cause of my issue as far as I can tell.

Are you able to open an issue on the eloquent-driver and we can look into it further.

[nadinengland]

I certainly can but before I do, and just to be totally clear, the issue is in the statamic/cms repo and is unrelated to the eloquent driver that you support. I am not using the Eloquent driver supported by Statamic, but rather a custom driver as mentioned, apologies for not making that clear.

The bug I mentioned is on line 438 of statamic/cms/src/Entries/Entry.php:

        if (empty($ids = $page->flattenedPages()->pluck('id'))) {
            return;
        }

This code will never return true as empty() always returns false if given an object. blank() should be used in this use case.