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

Undeprecate token parameter from vocabularies endpoint #1697

Closed
tisto opened this issue Aug 25, 2023 · 9 comments
Closed

Undeprecate token parameter from vocabularies endpoint #1697

tisto opened this issue Aug 25, 2023 · 9 comments
Milestone

Comments

@tisto
Copy link
Member

tisto commented Aug 25, 2023

Passing a token parameter to the vocabularies endpoint has been deprecated:

One should use "tokens" instead.

@tisto tisto added this to the 9.x.x milestone Aug 25, 2023
@tisto
Copy link
Member Author

tisto commented Aug 25, 2023

@sneridagh @davisagli I am unsure if we can remove allowing passing "token" instead of "tokens" parameter. Can you folks check if we do this properly in Volto 15, 16 and 17?

@tisto
Copy link
Member Author

tisto commented Aug 25, 2023

@davisagli
Copy link
Member

@tisto That particular code just passes through what was passed in to the request (token or tokens or both). You have to look at where that redux action is used

@tisto
Copy link
Member Author

tisto commented Aug 27, 2023

@sneridagh could you have a look at what we currently use in Volto 16 and 17? token or tokens? Dependent on what we use I'd make a decision on what to do...

@sneridagh
Copy link
Member

@tisto we allow both, for backwards compat.

https://github.com/plone/volto/blob/b703425a07b4830bcdb58fbef8f0f25a0ddf5489/src/actions/vocabularies/vocabularies.js#L65

I don't know if we should remove the dual code from Volto as well... I'd leave it.

@tisto
Copy link
Member Author

tisto commented Aug 28, 2023

@sneridagh I'd say we remove "token" from plone.restapi 9 and deprecate it in Volto 17. We can then remove it in Volto 18. Otherwise, we risk incompatibilities between Volto and plone.restapi that are unnecessary.

@davisagli
Copy link
Member

@sneridagh @tisto Currently volto can use either token or tokens, depending on whether the value is an array or not. I would personally prefer to keep supporting both in plone.restapi. I don't see a compelling reason to remove one of them. It would break compatibility of plone.restapi 10 with volto <17, and just make extra work for ourselves.

@sneridagh
Copy link
Member

I also share the same feeling as David... it does not hurt, we document/use in code (the locations are also very localized, meaning they are not spreaded everywhere) just one.

@tisto tisto changed the title Remove deprecated token parameter from vocabularies endpoint Undeprecate token parameter from vocabularies endpoint Sep 21, 2023
@davisagli
Copy link
Member

Done in #1708

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

No branches or pull requests

3 participants