enonic / xp

Enonic XP
https://enonic.com
GNU General Public License v3.0
201 stars 34 forks source link

Make schedule optional on PublishRequestIssue #7271

Closed sgauruseu closed 5 years ago

sgauruseu commented 5 years ago

Steps to reproduce:

  1. Open new folder wizard , type a name.
  2. Select Publish... menu item.

image

Caused by: java.lang.NullPointerException: text at java.base/java.util.Objects.requireNonNull(Objects.java:246) at java.base/java.time.format.DateTimeFormatter.parse(DateTimeFormatter.java:1945) at java.base/java.time.Instant.parse(Instant.java:395) at com.enonic.xp.admin.impl.rest.resource.content.json.PublishScheduleJson.(PublishScheduleJson.java:18) at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490) at com.fasterxml.jackson.databind.introspect.AnnotatedConstructor.call(AnnotatedConstructor.java:124) at com.fasterxml.jackson.databind.deser.std.StdValueInstantiator.createFromObjectWith(StdValueInstantiator.java:283) ... 95 common frames omitted

GlennRicaud commented 5 years ago

@pmi: I do not thing this is the problem. This parameter is mandatory. Please revert this but set the attribute "required" of the parameter "publishFrom" to true. @JsonProperty(value = "from", required = true)

alansemenov commented 5 years ago

@pmi publishFrom can never be null. if it's not set then we set it to Now() (that's how it used to be before as well)

pmi commented 5 years ago

@alansemenov @GlennRicaud ok I will add required and set publishFrom to Now() if no schedule is set.

But there are other things here: I can send entire PublishSchedule == null to publish content endpoint, which will publish it, but will give errors later when listing it. That is not the case with create issue endpoint where null PublishSchedule is not allowed.

BTW, should publishFrom also be set to now() for publish requests ?

GlennRicaud commented 5 years ago

"but will give errors later when listing it" when listing what?

GlennRicaud commented 5 years ago

So from what I understand here, the things to do in XP:

alansemenov commented 5 years ago

I suggest we follow the standard practice of pull requests for changes in this epic (only in this repo, not necessary for lib-admin-ui and app-contentstudio)

pmi commented 5 years ago

Okay,

Here is the PR to XP https://github.com/enonic/xp/pull/7278

And PR to CS since CS changes are dependent on XP ones https://github.com/enonic/app-contentstudio/pull/705