Currently the published_at value is set to some date in the far future for entities that have never been published. This makes it necessary to override the value whenever we want to display it or index it.

We already have a computed property published_at_or_now that returns the publication date, or the current time when the entity has never been published. Let's also add variants for published_at_or_created and published_at_or_updated that can be used in cases where it is desirable to use either the created date or the last updated date for entities that are not yet published.

CommentFileSizeAuthor
#3 3066317-3.patch3.18 KBpfrenssen
#2 3066317-2.patch4.08 KBpfrenssen
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pfrenssen created an issue. See original summary.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen at Randstad Digital for European Commission and European Union Institutions, Agencies and Bodies commented
Status: Active » Needs review
FileSize
3066317-2.patch4.08 KB
pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen at Randstad Digital for European Commission and European Union Institutions, Agencies and Bodies commented
FileSize
3066317-3.patch3.18 KB

This is a version of the patch which does not include coding standards fixes, so that it doesn't conflict with the patch from #2983348: Published on empty for existing nodes.

idimopoulos’s picture

idimopoulos CreditAttribution: idimopoulos at Randstad Digital for European Commission and European Union Institutions, Agencies and Bodies commented 
+++ b/src/PublishedAtOrCreatedComputed.php
@@ -0,0 +1,35 @@
+    if ($entity instanceof NodeInterface) {
+      return $entity->getCreatedTime();
+    }

+++ b/src/PublishedAtOrUpdatedComputed.php
@@ -0,0 +1,35 @@
+    if ($entity instanceof EntityChangedInterface) {
+      return $entity->getChangedTime();
+    }

I only have one concern about this. In new entities, are the 'created' and 'changed' properties when these two clauses are called? Otherwise it might return NULL.

Also, why are we checking for the entity type? This module only adds this field in nodes.

Edit: I still have an issue with the way this forces the users to handle the 'published_at' date. So according to the 3 computed fields + the default value, the published_at field will either get the request time, the created time, the changed time, or a date from 2032. Why not null? An empty publication date will mean that the entity was never published yet.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen at Randstad Digital for European Commission and European Union Institutions, Agencies and Bodies commented

I only have one concern about this. In new entities, are the 'created' and 'changed' properties when these two clauses are called? Otherwise it might return NULL.

That seems like the intended behavior to me, unsaved entities have no created, updated, or published date, so returning NULL is fine.

Also, why are we checking for the entity type? This module only adds this field in nodes.

This is just a good practice for object oriented code. We should not put the responsibility of checking the passed properties on the calling code. There is nothing preventing other code from instantiating this class.

Edit: I still have an issue with the way this forces the users to handle the 'published_at' date. So according to the 3 computed fields + the default value, the published_at field will either get the request time, the created time, the changed time, or a date from 2032. Why not null? An empty publication date will mean that the entity was never published yet.

That is a very good suggestion and I agree, but that is a change in the data model which should be handled in a separate issue.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen at Randstad Digital for European Commission and European Union Institutions, Agencies and Bodies commented

I created an issue for discussing the proper returning of NULL for unpublished content: #3066446: Field value should be empty for entities that were never published.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea as a volunteer and at Webikon for European Commission and European Union Institutions, Agencies and Bodies commented
Status: Needs review » Closed (won't fix)

I don't think a contrib module should provide support for business use cases. Those 2 new properties, and published_at_or_now as well, are smelling as trying to cover some needs from projects where publication_date is used. Instead, those projects should extend the field in a custom module or resolve the "OR" in other way. This is a "won't fix", IMHO.

EDIT: I'm also advocating for removal of published_at_or_now, in #3076541: Remove published_at_or_now (or move it to a submodule).