plone.restapi
plone.restapi copied to clipboard
plone
@site and @navroot endpoints
@erral 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!
Deploy Preview for plone-restapi canceled.
| Name | Link |
|---|---|
| Latest commit | 4b0626ac8559ee7b0877a769364d17e6b77dd2ef |
| Latest deploy log | https://app.netlify.com/sites/plone-restapi/deploys/647a423d96caf300082c90c0 |
![netlify[bot] avatar](https://avatars.githubusercontent.com/in/13473?v=4 "Published by a pub.dev verified publisher"){kind=small}
Sorry, but @site just returns a title? I would expect the whole site object information. Same for @navroot (even if it comes witth an URL). If its only the title I would name the endpoint @site-title or alike (similar for navroot).
I initially thought of @site to have site-wide settings but at this point I see that I only need the title. In the @navroot we at least need the url and the title
@cekk I have added some more settings to the @site endpoint:
- Title
- Logo URL (if defined)
- robots_txt
- image allowed sizes
I have a PR prepared in Volto that uses the logo defined in Plone to render it in Volto (https://github.com/plone/volto/pull/3537), I guess that using the robots.txt would not be that hard too.
Meanwhile I found a bug in plone.app.layout that does not correctly compute the navigation root title for the Plone Portal Object, and I am handling that there.
cool. I can improve it with logo miniatures (the only missing thing) but i don't know if it's better to add it to this pr or wait to close this and add e new one.
cool. I can improve it with logo miniatures (the only missing thing) but i don't know if it's better to add it to this pr or wait to close this and add e new one.
You can push to this branch if you want, no problem with that.
Ok, my changes are more complexes than i remembered. I need to register some adapters and customize @@images view to work on the site root and logo field. I don't think that these could be changes that should be in restapi package, right? I should split them in several packages and i don't know if it will be ok.
Maybe the easiest way could be let this pr as is and customize @site endpoint in plone.volto
@erral I just merged a reorganization of docs PR. See https://github.com/plone/plone.restapi/pull/1490 for a complete description, and https://github.com/plone/plone.restapi/pull/1490/files#diff-b1ac7029db5750e88b86b3cb22ffeb9a839080d7759471090d8c1eea349a9a02 for an example of necessary changes.
- The two new endpoints docs files need to be moved into
docs/src/endpoints - The paths to their tests need to be adjusted with another
../. - The meta HTML in the new endpoint docs needs to be tweaked.
- The relevant
index.mdfiles need to be updated.
Please let me know. Thank you!
ok i will have a look. I see that I have something to fix in one tests because of unpredictable UIDs, so I will look at it together
I had to do some changes to the test setup to try to get the tests pass, I would like a review from someone who understands how the component registry stuff works.
We had a StaticUUIDGenerator that was pushed into the component registry in the setUp method of the documentation tests in order to have always predictable UUIDs to be dumped to the response files.
I had to move that registration into the layer configuration. The reason is that when showing the tests of this PR in the documetnation, I needed to show the UUID of a LRF in a multilingual site, and this LRF was created before the StaticUUIDGenerator was being registered, so each time I run the tests I got different UIDs for the LRF objects.
The changes are here:
Previous: https://github.com/plone/plone.restapi/pull/1465/files#diff-702e9e8621c4eb2b19904313c62308cdbf458b8290e0ff261b779ceaa9101b90)https://github.com/plone/plone.restapi/pull/1465/files#diff-702e9e8621c4eb2b19904313c62308cdbf458b8290e0ff261b779ceaa9101b90
My changes: https://github.com/plone/plone.restapi/pull/1465/files#diff-89c6e068b94d1fadf3e6d6da1706e489800b7ba147946fc647ed9ae1c093799c
This has the drawback that I had to recreate all the response files to reflect the new UIDs there.
Moreover I have one testing failure in local_roles serializer: https://github.com/plone/plone.restapi/blob/site-endpoint/src/plone/restapi/tests/test_content_local_roles.py#L583
This is somehow expected, according to the docstring of that test, because the test manipulates de global component registry unregistering an adapter and then re-registering it. I don't know how this works and why it fails.
I think that either @lukasgraf or @buchi wrote that StaticUUIDGenerator and its pushing to the component registry, but I haven't see them lately here... Perhaps @davisagli has played with something like that in the past?
@stevepiercy sorry, I need to look again to the docs and your reviews, do not review them again. After the merging of the new navigation I may be missing stuff or may have missed some of your comments.
@stevepiercy sorry, I need to look again to the docs and your reviews, do not review them again. After the merging of the new navigation I may be missing stuff or may have missed some of your comments.
OK, please ping me when you are ready for me to review. Thank you!
I have reverted the changes on the testing setup, and commented the code that tests the endpoint in the LRF.
I think we should check the testing setup to produce stable UUIDs at all times in some other PR.
I saved the code in another branch, and try to push and test it in a separated PR.