Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

properly handle SiteTree objects #80

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

oetiker
Copy link
Contributor

@oetiker oetiker commented Jun 24, 2017

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.

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
Copy link
Owner

colymba commented Jun 25, 2017

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
Copy link
Contributor Author

oetiker commented Jun 25, 2017

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
Copy link
Owner

colymba commented Jun 25, 2017

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
Copy link
Contributor Author

oetiker commented Jun 25, 2017

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
Copy link
Owner

colymba commented Jun 25, 2017

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
Copy link
Owner

colymba commented Jun 26, 2017

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
Copy link
Contributor Author

oetiker commented Jun 26, 2017

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
Copy link
Owner

colymba commented Jun 27, 2017

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....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants