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

Working copy for Plone Site #5284

Open
wesleybl opened this issue Oct 6, 2023 · 19 comments · May be fixed by #6393
Open

Working copy for Plone Site #5284

wesleybl opened this issue Oct 6, 2023 · 19 comments · May be fixed by #6393

Comments

@wesleybl
Copy link
Member

wesleybl commented Oct 6, 2023

Is your feature request related to a problem? Please describe.
The portal home (Plone Site) is probably where the working copy would be most important. However, it doesn't work on home. It would be interesting to have a working copy for home.

Describe the solution you'd like
Perhaps the copy object could be a Document, with Plone Site blocks.

PRs that fix this issue in Plone 6.0:

#6393
plone/plone.restapi#1823
plone/plone.app.iterate#128
plone/Products.CMFEditions#113

@stevepiercy
Copy link
Collaborator

@alexialg05 I don't think so, as the demo site appears not to have it enabled by default. You can install Plone locally, then follow Working copy support.

Also please read and follow First-time contributors, especially Things not to do, Contributing to Plone, and Contributing to Volto.

@plone plone deleted a comment from alexialg05 Mar 24, 2024
@wesleybl
Copy link
Member Author

@alexialg05, @stevepiercy is correct. It is not enabled in the demo. You can do this locally. To see:

You need to enable working copy support in Volto's configuration object:

@djay
Copy link
Member

djay commented Apr 30, 2024

it's a bug not a enhancement. Working copy is broken without this and is a regression from classic.

@wesleybl
Copy link
Member Author

For this to work in Plone Site, you would need to enable versioning in it. See:

https://github.com/plone/plone.app.iterate/blob/1908aa1f4e9c95c37143fae88655469b72ea451a/plone/app/iterate/browser/control.py#L51-L52

@mauritsvanrees @davisagli @jensens do you see any problem with this?

@mauritsvanrees
Copy link
Member

Working copy of Plone Site sounds tricky, but enabling versioning behavior should be fine. I did not try it though.

@wesleybl
Copy link
Member Author

Working copy of Plone Site sounds tricky

I was trying to create a Plone Site inside another one. But I realized that this was getting weird. Even if I managed to do this, I would still have permission issues. Usually the user who can change the Portal cannot create a new Portal. So I changed the approach, to create a Document as a working copy of the Portal. Then I copy the fields from the Portal into the Document. Does this look ok?

@wesleybl
Copy link
Member Author

wesleybl commented Oct 1, 2024

I managed to create a working copy of the portal using Document as the working copy. I had to change 3 packages:

plone.app.iterate
Products.CMFEditions
plone.restapi

It seems to be working. Can I create the PRs?

@mauritsvanrees
Copy link
Member

Could be interesting, yes. I may not have time to review this though.

But what if the Plone Site and Document types are too different? Maybe they have been customised, and both have extra fields and behaviors, but not the same ones.

Or what if you create a working copy and then add some images in there, referenced by blocks. Do those get taken over when checking the document in? I guess that could work though, assuming that this currently works for standard folders, or indeed plone.volto Documents.

@wesleybl
Copy link
Member Author

wesleybl commented Oct 2, 2024

But what if the Plone Site and Document types are too different? Maybe they have been customised, and both have extra fields and behaviors, but not the same ones.

@mauritsvanrees All fields that Plone Site and Document have in common will be copied from Document to Plone Site on checkin. The Plone Site schema is used as a base. See:

https://github.com/plone/plone.app.iterate/blob/01dca2a0c95df2a712dbf1900bc51370a77fb03d/plone/app/iterate/dexterity/copier.py#L227

In the code above, baseline would be Plone Site and self.context would be Document.

Fields that are not common would remain unchanged. However, the main fields of interest for the working copy would be the fields related to blocks. And these fields are common. So I think it would be fine like this.

Or what if you create a working copy and then add some images in there, referenced by blocks. Do those get taken over when checking the document in? I guess that could work though, assuming that this currently works for standard folders, or indeed plone.volto Documents.

Yes it works.

@davisagli
Copy link
Member

How badly would things break if the working copy was an instance of Document with its portal_type set to "Plone Site" so that it uses the Plone Site schema?

I'd be interested to review the implementation, but can't promise to get to it quickly.

@wesleybl
Copy link
Member Author

wesleybl commented Oct 2, 2024

How badly would things break if the working copy was an instance of Document with its portal_type set to "Plone Site" so that it uses the Plone Site schema?

@davisagli I don't know if I understood your suggestion very well. You say to dynamically change the portal_type from Document to Plone Site, after creating the copy? Would that work? I'll test it.

@davisagli
Copy link
Member

@wesleybl That's right. I'm not sure if it will work, but it might.

@wesleybl
Copy link
Member Author

wesleybl commented Oct 2, 2024

That's right. I'm not sure if it will work, but it might.

@davisagli This worked! When editing the working copy, only the Plone Site fields appeared. The only difference is that the working copy did not appear in the navigation menu, since Plone Site does not appear there. But I don't think that's a problem.

Thanks for the suggestion!

@wesleybl
Copy link
Member Author

wesleybl commented Oct 3, 2024

@djay @mauritsvanrees @davisagli I created the following PR:

With these PRs, you can now test the working copy in Plone 6.0 Classic. You will need to create a new Plone Site.

There are missing PRs in restapi and Volto to make it work in Volto.

@wesleybl
Copy link
Member Author

@djay @mauritsvanrees @davisagli I've made PRs on plone.restapi and Volto. This allows you to test the changes on Volto. I've included the PRs in the issue description. Please test them when you have time. Thanks!

@mauritsvanrees
Copy link
Member

I must say I am having a doubt about including this in Plone 6.0, as this needs changes in no less than four backend packages. Maybe it is okay. At least some of the PRs are small and safe enough that it seems fine to make the change. But if you would be okay with targeting this at Plone 6.1 only, I would be happier.
I am at this point not yet saying no to 6.0, just expressing a doubt. I have not tried it out, nor viewed all PRs.

For 6.1 it would be good to have this if it works. I have added a link here.

Could you add a PLIP configuration for 6.1 in buildout.coredev? There are some examples in the plips directory. Just add the relevant packages to auto-checkout and add [sources] with the branches. That makes it easier to try out.

Finding time for a proper review is hard currently. And I don't want this to really hold up a 6.1 beta release. But I would say it should be fine to include this a bit later in the beta stage still.

@wesleybl
Copy link
Member Author

wesleybl commented Nov 4, 2024

But if you would be okay with targeting this at Plone 6.1 only, I would be happier.

I would like to have this in Plone 6.0 too. I think it would be safe.

Could you add a PLIP configuration for 6.1 in buildout.coredev?

Done in plone/buildout.coredev#966

@davisagli
Copy link
Member

Current status (/cc @wesleybl @mauritsvanrees):

  • Allows working copy of Plone Site plone.app.iterate#130 has changes to support checkin/checkout for the Plone Site by creating the working copy as a document stored inside the portal root. I also just added a change to support working copies in general for content types that don't support versioning.
  • That means we no longer need the changes in CMFEditions. (We can revisit them later to try to support versioning for the Plone Site, but that will require more significant work.)
  • Allows working copy of the Portal plone.restapi#1823 adds working copy info to the serialization of the portal root, when it is available from plone.app.iterate.
  • Show working copy links based on backend actions #6393 updates the logic for when Volto displays the checkin/checkout actions, so that it's based on the @actions endpoint from the backend instead of a setting in the Volto config. This helps make sure the conditions are the same between Classic UI and Volto, and also makes sure we don't show them unless plone.app.iterate is installed.

So far this works in Plone 6.1. It should also work in Plone 6.0 if someone updates plone/plone.app.iterate#128

@mauritsvanrees
Copy link
Member

I did not try it in the Volto UI, but from a Classic UI viewpoint this is fine. Works fine for a Volto site when accessed through the Classic UI backend as well.

I don't think we should do the plone.app.iterate change in Plone 6.0 anymore though. It feels like too big a change this late in the game. 6.0 is strictly speaking out of maintenance support, although we should amend this to be extended until 6.1 final is out (when writing the policy, we expected that 6.1 would arrive sooner).

But if you know what you are doing, it is fine to use the latest plone.app.iterate release on 6.0 as well. It technically has a breaking change because it removes an old GS profile, but it feels minor. It should be fine to migrate a 5.2 site to 6.0.14 with the officially supported plone.app.iterate version, run all upgrade steps, and then start using the latest version.

The plone.restapi change is fine for 6.0 and 6.1.

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

Successfully merging a pull request may close this issue.

5 participants