plone.restapi icon indicating copy to clipboard operation
plone.restapi copied to clipboard

Always return an effective_date value and fall back to creation_date if no effective_date is set.

Open tisto opened this issue 7 years ago • 8 comments

plone.restapi currently returns the "raw" value for both effective_date and creation_date. Say you create a new object it will return:

effective_date: None, creation_date:

When a frontend client now queries the backend and wants to sort by effective_date (which is None in most cases), lots of frontend code complexity is necessary to match Plone's standard behavior of falling back to the creation_date value if effective_date is not set. This is what standard Plone returns (at least to the user):

effective_date: creation_date:

I don't have time right now to look into the implementation details of how Plone handles that internally. I suspect the logic is in the catalog or the templates.

Before I dig deeper into this I'd like to hear opinions.

Would you be ok if we handle this on plone.restapi level and make effective_date always return a value (if None is set in the UI, return the creation_date). This will drastically reduce the complexity we need on the frontend.

@buchi @lukasgraf @davisagli @sneridagh @ebrehault ?

@bloodbare @vangheem how does Guillotina currently handle this?

cc @robgietema

tisto avatar Sep 08 '18 10:09 tisto

I think this would complicate other things. For instance it would not be possible to see if there is no effective_date.

And: When publishing a page in plone (5.1.2), the effective date is set to current date/time if it is unset.

sunew avatar Sep 08 '18 11:09 sunew

I don't think Plone makes any assumption on effective date at the end it's the Catalog which sorts the search results, so if effective_date is not set and is None, the sort is based on that 'None' value.

AFAIK Plone sets effective_date when transitioning a content object to the 'published' state. Previously it was done using a workflow script nowadays I suspect there must be an event subscriber but I can't find it right now.

Moreover there was a discussion on this issue some time ago in community.plone.org: https://community.plone.org/t/shouldnt-effective-date-be-set-to-creation-date/1980

erral avatar Sep 10 '18 06:09 erral

@erral thanks for the pointer, this is helpful. It seems that the effective date is not set on a workflow transition in our system. Need to dig deeper I guess...

tisto avatar Sep 10 '18 06:09 tisto

@wkbkhard fixed effective date on workflow transitions in #761 (which replicates the logic in plone.app.content.browser.content.workflow).

effective() and expires()

effective date sorting is done with the return value fro plone.dexterity.content.DexterityContent.effective(). This method returns in this order

  • effective_date or
  • creation_date or
  • floor date (1970/1/1)
    @security.protected(permissions.View)
    def effective(self):
        # Dublin Core Date element - date resource becomes effective.
        date = getattr(self, 'effective_date', _marker)
        if date is _marker:
            date = getattr(self, 'creation_date', None)
        date = datify(date)
        return date is None and FLOOR_DATE or date

The same applies to DexterityContent.expires(). effective/expires are catalog indexes and a metadata fields. It can be serialized for search results.

name conflicts

The only problem is that effective has a name conflict. It is a method of DexterityContent and also a schema field of plone.app.dexterity.behaviors.metadata.IPublication. When we serialize dexterity content effective is already a property in the JSON:

@provider(IFormFieldProvider)
class IPublication(model.Schema):
 # ...
    effective = schema.Datetime(
        title=_(u'label_effective_date', u'Publishing Date'),
        description=_(
            u'help_effective_date',
            default=u'If this date is in the future, the content will '
                    u'not show up in listings and searches until this date.'),
        required=False
    )
    directives.widget('effective', DatetimeFieldWidget)
 # ...
class Publication(MetadataBase):
    effective = DCFieldProperty(
        IPublication['effective'],
        get_name='effective_date'
    )

csenger avatar Jun 21 '19 21:06 csenger

@tisto If we look at https://docs.plone.org/working-with-content/using-collections/oldstyle/using-and-understanding-dates.html EffectiveDate should be set when we publish an object. In your volto issue you've mentioned the following: "What happens in standard Plone is that if you publish an item the effective date index is set to now. We have to make sure this work similarly in Volto." https://github.com/plone/volto/issues/761 My issue with the changes made in this pull request https://github.com/plone/plone.restapi/pull/761 made because of https://github.com/plone/plone.restapi/issues/760 is that any workflow transition will trigger a setting of a DateTime to EffectiveDate which goes against the idea of having the publishing date set only when it's not set manually and the workflow state is Published.

Right now we have issues with this behavior in Volto when we trigger a transition and we get a publishing date although the object isn't ready to be published.

I would like to add a pull request to modify the behavior set here https://github.com/plone/plone.restapi/pull/761/commits/0052a5694ac1dcb75a27b6539a8066a51ea23948#diff-fe121724642283be623738f1a7c1ad0085ddbb1cc4f7c16f68492ea3baa0806dR109 to only trigger if the state id is published.

I am aware that plone 5 behaves wrongly as well from plone.app.content which got it's logic from plone.app.toolbar made by the same author https://github.com/plone/plone.app.toolbar/commit/531c51c3ebdad9bbd54ff82ea3338c68be22f09f#diff-c551bf8abba642716fa185d8b3f874cf9b220b4d1e7d9bfef179c4040aff1119R346 however I think that our initial documentations states that EffectiveDate should be set only when published and right now this is not the case.

This should probably be done in a way that you can enable or disable if you would like in order to keep the previous behavior or to get the effectiveDate automatically added on workflow transition only if that transition is to publish

ichim-david avatar Jul 15 '21 16:07 ichim-david

@ichim-david your comment makes lots of sense and it is indeed strange that any wf transition sets effective_date. Though, one could argue that an effective_date only makes sense on a published object and can otherwise be ignored. Even when other transitions set the effective_date, the effective_date is only taken into account on a published object.

What happens when an effective_date is set by a previous wf transition and then you publish the object? Is the effective_date overridden then with the last date as you would expect?

Say we would set the effective_date only on "publish", which would work for the standard workflows but not for a custom workflow that might have a different name for a public workflow state. Maybe that's, even more, an edge case.

Anyways. I asked Nathan in the commit for insights on why Plone behaves as it does right now:

https://github.com/plone/plone.app.toolbar/commit/531c51c3ebdad9bbd54ff82ea3338c68be22f09f#r53608588

tisto avatar Jul 17 '21 05:07 tisto

@ichim-david your comment makes lots of sense and it is indeed strange that any wf transition sets effective_date. > Though, one could argue that an effective_date only makes sense on a published object and can otherwise be ignored. Even when other transitions set the effective_date, the effective_date is only taken into account on a published object.

I would be in favor of removing the setting of effective_date from the transitions, at EEA we actually have some event handlers where we check if the review state is published and no effectiveDate is set then we set it to now.

What happens when an effective_date is set by a previous wf transition and then you publish the object? Is the effective_date overridden then with the last date as you would expect?

At EEA we have some overrides for the Plone 4.3 code and we check if an effectiveDate is set and if it is then we don't modify the publishing date when we reach published state. In our case it's the editors that set the publishing date for a certain time in case it want's to be published then and any code that would set an EffectiveDate if it's not added by the publish transition or manually by them is a bug. This is why one easy thing that we could do in the current code is to check is: if self.EffectiveDate() == 'none' and self.transition == 'publish' This way only if there is no date set and the transition is published do we set one fulfilling the premise of having an effectiveDate when we publish and we had no date set.

Say we would set the effective_date only on "publish", which would work for the standard workflows but not for a custom workflow that might have a different name for a public workflow state. Maybe that's, even more, an edge case.

Anyways. I asked Nathan in the commit for insights on why Plone behaves as it does right now:

plone/plone.app.toolbar@531c51c#r53608588

One possible solution would be to remove this date setting from the transition code and have the transition id's customizable either from a config file that can be ready by plone.restapi or registering an adapter that could provide other values that would the be checked.

I am also thinking if this check isn't better done in an event handler instead of the transition code.

ichim-david avatar Jul 18 '21 14:07 ichim-david

Anyways. I asked Nathan in the commit for insights on why Plone behaves as it does right now:

plone/plone.app.toolbar@531c51c#r53608588

I am getting a headache trying to follow this behavior but I have found the following:

  1. Even from plone 2 within content_status_modify.py if no date is set then an effective date is set https://github.com/plone/Products.CMFPlone/commit/e89d135d6324473b8456182edfc1f067b059fd08#diff-91ca25a875245c61a8120c8259df39a3f8ed54171ed2fdeb7e7c8dcdfb196e05R13
  2. Products.Archetypes EffectiveDate description states the following: "The date when the item will be published. If no date is selected the item will be published immediately" https://github.com/plone/Products.Archetypes/blob/1.12/Products/Archetypes/ExtensibleMetadata.py#L134

If we read the summary we can deduct that all of this code then does what it's suppose to do which is to set an effective date if no effective date is set manually. The difference is though the second part of the description because yes the object receives an effective date but it's review state isn't changed to published so we have a nomenclature and behavior difference.

ichim-david avatar Jul 20 '21 18:07 ichim-david