nephila / djangocms-page-sitemap

django CMS page extension to handle sitemap customization
BSD 3-Clause "New" or "Revised" License
9 stars 21 forks source link

Updated lastmod() to return latest publish version modified date in cms4 #58

Closed NarenderRajuB closed 4 years ago

NarenderRajuB commented 4 years ago

Lastmod() currently returns the page changed_date. If djangocms_versioning is installed and enabled on cms4, then lastmod key for the PageUrl in sitemap, should return the pagecontent's latest version modified date.

  1. Test has been added for the change , but at present we are skipping as this may require change in test suite to make it compatible with djangocms_versioning.
coveralls commented 4 years ago

Coverage Status

Coverage remained the same at 0.0% when pulling 08ed1338874c01a77008e81f68a250892858143d on NarenderRajuB:feature/lastmod_update_for_cms4_versioning into a85999e400933ecef12b3341ef19b984a2a497ee on nephila:support/4.0.x.

Aiky30 commented 4 years ago

@yakky Hello, As you can see in this PR we ran into an issue when using the new djangocms_versioning package that is for django CMS 4+. To be able to run the test for versioning we would need to change every test that creates a page because when versioning is installed all created pages are drafts by default. Before we did the extensive job of amending the test we need to know how you wish to proceed.

The options that I can see:

Thanks, Andrew

NarenderRajuB commented 4 years ago

@yakky Hello, As @Aiky30 mentioned above options we have for this PR , can you update us how we can proceed. As this is required fro django CMS4+ with versioning.

yakky commented 4 years ago

@Aiky30 @NarenderRajuB for now I think it's fine not to have test for this

I want to understand better how djangocms-versioning work before doing the extensive tests refactoring

Aiky30 commented 4 years ago

@Aiky30 @NarenderRajuB for now I think it's fine not to have test for this

I want to understand better how djangocms-versioning work before doing the extensive tests refactoring

@yakky Is it best to keep the test that Naren has created with a skip that states that versioning is currently an optional component so that in the future we can then turn this test on? We can also do a conditional skip if versioning is not installed, that means anyone that does use versioning can run the test suite locally?

Versioning documentation: https://github.com/divio/djangocms-versioning/blob/master/docs/index.rst

yakky commented 4 years ago

@Aiky30 @NarenderRajuB I think we can keep it for now with the conditional skipping to allow to run locally and as a reference for future test changes

Aiky30 commented 4 years ago

@yakky Narender has made the changes. Please let us know if you are happy / unhappy with anything.

Thanks.

NarenderRajuB commented 4 years ago

@yakky can you let us know if you are happy with the changes. so that we can have version with the changes

yakky commented 4 years ago

0.9.0dev4 uploaded to divio cloud

Aiky30 commented 4 years ago

@yakky Thank you you're more than helpful. We are very grateful for your time and patience.

Have a nice weekend. :-)

NarenderRajuB commented 4 years ago

@yakky . Thank you