Publishing a page with the toggle visibility button doesn’t update the sitemap.xml

[ausi]

If you publish a page with the publish/unpublish button the sitemap.xml does not get rewritten. I think this can be fixed by calling $this->updateSitemap() in the tl_page::toggleVisibility() method.

[leofeyer]

Hm, it definitely should: https://github.com/contao/news-bundle/blob/master/src/Resources/contao/dca/tl_news.php#L1034-L1037

[Toflar]

Can we try to remove the sleep() call anyway? That absolutely makes no sense :-D

[leofeyer]

You can try, but

for some reason it does not work without sleep(1)

[ausi]

Your link goes to tl_news, but the problem I have is with tl_page, see tl_page.php:1780 a call to $this->updateSitemap() is missing there.

[leofeyer]

You are right. The sitemap update is triggered in on of the onsubmit callbacks, however we are only executing the save callbacks of the field. @contao/developers Should we also execute the onsubmit callbacks or is it sufficient to simply add $this->updateSitemap()?

[ausi]

I agree that it might be a good idea to execute the onsubmit callbacks so that third party callbacks on tl_page get executed too. But maybe we should add a check for $dc not being null then, because the onsubmit callbacks depend on the $dc parameter AFAIK.

[DanielSchwiperich]

running the onsubmit callback stack is out of context to me when changing one field via a different functionality.

I would vote (if I could 😃 ) to just add the updateSitemap method to save callback of the field.

[leofeyer]

@DanielSchwiperich I get your point. But the onsubmit callbacks would also run if you edited the record and only changed the one field, wouldn't it?

[Toflar]

To me, the onsubmit_callback should be triggered as well.

[DanielSchwiperich]

yes leo, there's also no reasonable scenario coming to mind where this should not be ok.

[leofeyer]

Fixed in a46f8ac. The changes worked in my test setup, still I encourage you to review them thoroughly.

[ausi]

I’m not sure about the places with ($dc ?: $this) because $this is not a DataContainer. If my callback has a DataContainer type hint against the $dc parameter it would fail, shouldn’t we just skip the callbacks if $dc is null?

[leofeyer]

Do you remember why we have added ($dc ?: $this) in the first place? See contao/core#7314

[ausi]

OK, so if i understand it correctly, $dc is always set unless someone calls the method in his own project/extension and $this is then only passed for BC reasons, right?

[leofeyer]

Yes, I think so.

[fatcrobat]

@leofeyer I know this issue was already closed, but running tl_page::updateSitemap as onsubmit_callback which will trigger Automator::generateSitemap will also trigger each $GLOBALS['TL_HOOKS']['getSearchablePages'] hook. We have customers with a lots of news (5000+) that will result in hight memory consumption and increased page load. Is the sitemap generation not something that should done by with a cron job?