colymba / silverstripe-restfulapi

SilverStripe RESTful API with a default JSON serializer.
BSD 3-Clause "New" or "Revised" License
64 stars 34 forks source link

properly handle SiteTree objects #80

Open oetiker opened 7 years ago

oetiker commented 7 years ago

when changing a page, we have to use writeToStage and publish ... when using just 'write' the page will get a modified label in the cms, but the changes will not be visible ...

what might be interesting, is to provide an argument for PUT/POST to decide if the record should be published or just staged.

colymba commented 7 years ago

Thanks, definitely a needed addition. It would be better to test if the model has the Versioned extension, so it works for SiteTree and any other models.

I think this is a good base, but like you said we can look at adding a Stage query parameters, that can then be used in post, put but also maybe delete?

oetiker commented 7 years ago

yes ... detecting Versioned makes sense (how do you do that ?)

I also thought that maybe in absence of explicit instructions it would be best to detect of a page is stages or published and then update either the Staged or Live version ... and not change the publication state ...

colymba commented 7 years ago

hasExtension() should do the trick: https://docs.silverstripe.org/en/3/developer_guides/extending/extensions/#checking-to-see-if-an-object-has-an-extension

Not sure about the 'default', maybe just a write action is enough, which should use the current default Stage anyway. Then probably best to have 2 query parameters, Stage and Publish, which take the Stage to write to and the Stage to publish to. At least for a start, since we can't assume the stage names as they can be user defined I think.

oetiker commented 7 years ago

That would have to be __stage and __publish for name spacing reasons.

"Just write" does not work, at least not when I tried it ... the Page gets a 'modified' marker in the cms ui, but when looking at the content, it is unchanged ... when reading the page again via REST, all the changes are present, but in the cms ui, the old values are still there ...

that is how I got it into my head to fix it :)

colymba commented 7 years ago

Oh yeah I remember seeing this happen for pages, I think it might be just an issue of which stage is being read...

Anyway, am also thinking we might be able to handle all the Versioned logic earlier in the query handler and all in one place, something to do with Versioned::set_reading_mode which would save some duplicate work in get, post, put queries....

colymba commented 7 years ago

I think we might be able to have the whole Versioned login in handleQuery() just before switch ($request->httpMethod()).

There we can check if $model hasExtension('Versioned'). if yes use Versioned::set_reading_mode() to the __Stage query param. And this should affect any following read and write...

And if __Stage is not set, let's default to Live

oetiker commented 7 years ago

generic Versioned support is cool ... although for my usecase I really need the following behavior:

if the page has been edited in the cms, and is thus in stage mode, then I would not want it published, if has been published, then I would want to modify the published version.

if the page has been 'un-published' I would want to edit the un-published version, but I guess that is the same as if the paget is in stage mode ...

any advice on how to achieve this ?

colymba commented 7 years ago

I think in most cases, requests should include the __Stage param to avoid unexpected results.

But like you mention, maybe default to Live might not be wise, since in some case we might not want to publish.

Good thing is that Versioned has a latestPublished method that will let us know if the latest version is published of not. So with that, if there are no explicit __Stage param, we can edit the Live version if it is published otherwise the Stage one. And that should do the trick....