plone
Volto blocks editable layout behavior
Deploy Preview for plone-restapi canceled.
| Name | Link |
|---|---|
| Latest commit | 99c229c4cb2b59cbd4b8c2ef3426e12b3bd101ad |
| Latest deploy log | https://app.netlify.com/sites/plone-restapi/deploys/633ea856e7d733000899f0c8 |
![netlify[bot] avatar](https://avatars.githubusercontent.com/in/13473?v=4 "Published by a pub.dev verified publisher"){kind=small}
@avoinea thanks for creating this Pull Request and helping to improve Plone!
TL;DR: Finish pushing changes, pass all other checks, then paste a comment:
@jenkins-plone-org please run jobs
To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically.
Happy hacking!
@davisagli @tisto could you please take a look? I think @avoinea needs it.
@sneridagh @avoinea It seems a little strange to add this in plone.restapi, since someone could be using plone.restapi without using Volto at all. (It's also strange to have the existing IBlocks behavior here, but I guess that ship already sailed.) Could it go in plone.volto instead?
On the other hand, I guess it doesn't really make a difference since plone.volto is now a dependency of Plone 6 and behaviors will show up whether or not the plone.volto profile has been installed.
I guess I'm okay going ahead with this since I don't have a better proposal at the moment.
On the other hand, I guess it doesn't really make a difference since plone.volto is now a dependency of Plone 6 and behaviors will show up whether or not the plone.volto profile has been installed.
This s not true, you can install CMFPlone without Volto, but with restapi.
I propose to add behaviors related to volto in plone.volto. The blocks behavior should go there as well at some point in time, but not before 6.0 final is released.
@jensens @davisagli I do agree that the blocks related behaviors should go into plone.volto package. But until we move them there, I think they should stay together, here.
I agree to keep this here for Plone 6.0. Thanks to behavior short-names its easy to move them around in future.
@avoinea btw., is there already an issue in either plone.volto or plone.restapi which covers this move out? If not would you mind to add one?
@jensens @davisagli when we implemented the IBlocks behavior there was no plone.volto package. We will have to move stuff over for Plone 6.1 or 7.
@avoinea Overall this LGTM. The thing I am wondering about is the naming. "Editable blocks" confuses me because all blocks in Volto are editable. I am not a native English speaker but isn't the functionality we are talking about rather a "template" or "page template"? So maybe "Blocks Page Template" or "Editable Page Template"? Just thinking out loud here. Options?
@tisto Actually it's called Blocks (Editable Layout):
name="volto.blocks.editable.layout"
title="Blocks (Editable Layout)"
And I avoided to call it editable blocks because of the reason you mentioned.
Now, as the functionality in ControlPanel is labeled Layout I wouldn't go into Page templates or something else :thinking:
Best we can do: Blocks (Editable Page Layout), but then: is this referring to the Page content-type or the generic WebPage :smile:
Naming is always a pain :smile:
Hmm. CI fails (https://github.com/plone/plone.restapi/actions/runs/3188037698/jobs/5200243129):
-Authorization: Basic YWRtaW46c2VjcmV0
+Authorization: Basic YWRtaW46Y29ycmVjdCBob3JzZSBiYXR0ZXJ5IHN0YXBsZQ==
weird... nothing has changed in that regard.
and it was green in https://github.com/plone/plone.restapi/pull/1477/commits/9180377904c34fd0c793d3c64fae5dd183be0128
@avoinea ?
@sneridagh Doesn't look related to this PR. Something may be changed on the Plone side.
That's super odd. Are we pinning Plone versions and all dependencies in our builds?
Right, we are using -latest in requirements.
@mauritsvanrees do you recall a change in b3 that could trigger this change?
https://github.com/plone/plone.restapi/pull/1477#issuecomment-1268126898
It seems that the serialization of the Basic auth has changed... (it seems like a different hash type).
How many times do I have to tell folks not to do this: https://github.com/plone/plone.restapi/blob/master/requirements-6.0.txt
I created a PR to pin the versions properly: https://github.com/plone/plone.restapi/pull/1512
The minimum password length is now 8 characters instead of 5, and for example the default password in plone.app.testing was updated accordingly. Maybe that is it? Just a guess.
Looks like that's the culprit:
https://github.com/plone/plone.app.testing/commit/8f93055c933abd9d9f9265d36232051cbab455e6
@mauritsvanrees this breaks if you update Volto to b3: https://github.com/plone/volto/actions/runs/3189141686/jobs/5202614190
That means that it might break projects that do not use the Cypress autologin from core :(
I'd say it's quite a breaking change... at least from the testing environments out there. How about the old Robot tests that people might have in place? If they do not use the default auto login it will also break, right?
@mauritsvanrees @tisto @sneridagh Sorry, I didn't think about the fact that this would affect browser tests too.
It might be hard to avoid the breaking change, because increasing the minimum password length is an important security measure and I think that overrides the desire for backwards compatibility. But, I will look to see if there is some way to bypass the check for this password. And certainly we can use something more intuitive than "correct horse battery staple", which was me optimizing for compatibility with future increases of the minimum length, when I thought the password was only used within the Python tests.
In regard to the question about whether to test using -latest:
- I agree that builds should be repeatable, so we should use a specific version pin.
- However, this delays finding out when there is a breaking issue in the backend that only becomes obvious when testing the frontend against it, like this issue. Even when using -latest there is still a delay. To reduce the delay we need a CI build of the Plone backend that runs the frontend tests against it. This sound like something to mention in the "future" part of my talk on testing Plone next week...
Guys, sorry for the noise. I was the one who updated to latest.
I think it would be good to discover problems with this as soon as possible. In my view, it was good to see that now. But this does not seem to be unanimous. The specific Plone version ends up getting out of date.
Sorry.
