Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add last modified field value to sitemap when exists #7313

Merged

Conversation

martinlagler
Copy link
Contributor

@martinlagler martinlagler commented Mar 18, 2024

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Related issues/PRs #7238, #7313
License MIT

What's in this PR?

Adding the last modified field to the sitemap if it's defined.

Why?

For better content in the sitemap.

@martinlagler martinlagler changed the base branch from 2.5 to 2.6 March 18, 2024 08:29
@@ -126,7 +126,7 @@ private function generateSitemapUrl(
string $host,
string $scheme
) {
$changed = $contentPage['changed'];
$changed = $contentPage['lastModified'] ?? $contentPage['changed'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chirimoya is the new expected behaviour not returning the created instead of changed date of the node? Should we remove the fallback to changed? It is a little bit strange here now. As soon as once the lastModified is defined the changed is not longer automatically provided to the sitemap, is that expected?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed with @chirimoya to avoid a backwards compatibility break we will keep here the fallback to changed like currently ipmlemented. We maybe will change this in future when it is maybe possible in the UI of the publishing to set update lastmodified timestamp via a toggler.

@alexander-schranz alexander-schranz changed the title Feature/last modified sitemap Add last modified field value to sitemap when exists Mar 18, 2024
Copy link
Member

@alexander-schranz alexander-schranz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add a test where the lastModified field is filled.

@alexander-schranz alexander-schranz added the Feature New functionality not yet included in Sulu label Mar 18, 2024
@alexander-schranz alexander-schranz added this to the Release 2.6 milestone Mar 18, 2024
@alexander-schranz alexander-schranz merged commit f026886 into sulu:2.6 Mar 22, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New functionality not yet included in Sulu
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants