search

plone / plone.restapi

RESTful API for Plone.
http://plonerestapi.readthedocs.org/
84 stars 73 forks source link

Support getting /@querystring vocabs in context #1704

Closed davisagli closed 9 months ago

davisagli commented 9 months ago

Add support for getting the /@querystring endpoint in a specific context, with vocabularies in that context.

This depends on https://github.com/plone/plone.app.querystring/pull/137 for getting the vocabularies in the specified context. (But the new endpoint will still work without the change in plone.app.querystring, only with the vocabularies always in the context of the portal.)

I also edited the docs for this endpoint a bit.

mister-roboto commented 9 months ago

@davisagli 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!

netlify[bot] commented 9 months ago

Deploy Preview for plone-restapi canceled.

Name Link
Latest commit 6819beea9555b7c0c540d435b52e440feae9b4fc
Latest deploy log https://app.netlify.com/sites/plone-restapi/deploys/650c8a743a23900008ba4e9a
davisagli commented 9 months ago

@jenkins-plone-org please run jobs

davisagli commented 9 months ago

@jenkins-plone-org please run jobs

tisto commented 9 months ago

@jenkins-plone-org please run jobs

tisto commented 9 months ago

@davisagli: @mauritsvanrees asked me to wait with a plone.restapi 9 release. Therefore we might want to backport this feature to the plone.restapi 8.x.x branch to being able to move on...

tisto commented 9 months ago

@davisagli @ericof @sneridagh seems the plone.restapi build pulls in "latest" somewhere. We have a test failure now that is completely unrelated to this PR:

Failure in test test_get_own_user_portrait_with_svg (plone.restapi.tests.test_services_users.TestUsersEndpoint)
Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.7.17/x64/lib/python3.7/unittest/case.py", line [59](https://github.com/plone/plone.restapi/actions/runs/6262417794/job/17004534477?pr=1704#step:10:60), in testPartExecutor
    yield
  File "/opt/hostedtoolcache/Python/3.7.17/x64/lib/python3.7/unittest/case.py", line 628, in run
    testMethod()
  File "/home/runner/work/plone.restapi/plone.restapi/src/plone/restapi/tests/test_services_users.py", line 1110, in test_get_own_user_portrait_with_svg
    self.assertEqual(response.headers["Content-Type"], "image/svg+xml")
  File "/opt/hostedtoolcache/Python/3.7.17/x[64](https://github.com/plone/plone.restapi/actions/runs/6262417794/job/17004534477?pr=1704#step:10:65)/lib/python3.7/unittest/case.py", line 852, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/opt/hostedtoolcache/Python/3.7.17/x64/lib/python3.7/unittest/case.py", line 1233, in assertMultiLineEqual
    self.fail(self._formatMessage(msg, standardMsg))
  File "/opt/hostedtoolcache/Python/3.7.17/x64/lib/python3.7/unittest/case.py", line [69](https://github.com/plone/plone.restapi/actions/runs/6262417794/job/17004534477?pr=1704#step:10:70)3, in fail
    raise self.failureException(msg)
AssertionError: 'text/html; charset=utf-8' != 'image/svg+xml'
- text/html; charset=utf-8
+ image/svg+xml

I honestly don't know what else I need to say to convince you that fetching "latest" is just plain wrong...

tisto commented 9 months ago

@davisagli @sneridagh @ericof I removed "latest" from the buildout some time ago. Maybe @mauritsvanrees already fixed the test and now it is the other way around (test will pass for "latest-" Plone and currently fails for the old version). Investigating (and hoping my previous comment does not make me look like an idiot now ;))...

tisto commented 9 months ago

@davisagli @sneridagh @ericof seems @mauritsvanrees already fixed the tests in his PR, a manual upgrade to latest = Plone 6.0.7 fixes the failing test. Everything is as it should be, sorry for the noise. :)

mauritsvanrees commented 9 months ago

@tisto I have changed coredev 6.0 to checkout plone.restapi from the main branch again. If you want to make a release now, go ahead.

tisto commented 9 months ago

@jenkins-plone-org please run jobs