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

fix: do not validate locale if not creating #19626

Merged
merged 4 commits into from
Feb 28, 2024

Conversation

Marc-Roig
Copy link
Contributor

@Marc-Roig Marc-Roig commented Feb 28, 2024

What does it do?

I18n Backend koa middlweare was matching all CM routes, and validating locale creation even for actions that were not related (publishing, unpublishing)

If the API received the "relatedEntityId" param when publishing, it assumed this was for creating a new locale (and checked if the locale already existed), and failed because the action was to publish an existing locale.

My only worry here is that the url matching is not smart enough and we miss some cases

How to test?

Create single types and collection type locales, and publish them. You should not get an error

Related issues

@Marc-Roig Marc-Roig added source: plugin:i18n Source is plugin/i18n package pr: fix This PR is fixing a bug labels Feb 28, 2024
@Marc-Roig Marc-Roig added this to the 4.20.3 milestone Feb 28, 2024
@Marc-Roig Marc-Roig self-assigned this Feb 28, 2024
Co-authored-by: Josh <37798644+joshuaellis@users.noreply.github.com>
Co-authored-by: Ben Irvin <ben.irvin@strapi.io>
@Marc-Roig Marc-Roig linked an issue Feb 28, 2024 that may be closed by this pull request
Copy link
Contributor

@innerdvations innerdvations left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed, neither of us like this, but it fixes the problem and there doesn't appear to be a better way 😆

I suspect that even though this will solve 99% of the cases, there will be a handful of edge cases we didn't anticipate that will have to be solved later, but we'll find out then, and it shouldn't actually break anything

@Marc-Roig Marc-Roig removed the pr: fix This PR is fixing a bug label Feb 28, 2024
@Marc-Roig Marc-Roig changed the base branch from releases/4.20.3 to develop February 28, 2024 13:57
@Marc-Roig Marc-Roig changed the base branch from develop to releases/4.20.3 February 28, 2024 13:58
@joshuaellis joshuaellis added the pr: fix This PR is fixing a bug label Feb 28, 2024
@Marc-Roig Marc-Roig merged commit 91a9cca into releases/4.20.3 Feb 28, 2024
8 checks passed
@Marc-Roig Marc-Roig deleted the fix/i18n-can-not-publish-locale branch February 28, 2024 14:02
@minht11
Copy link

minht11 commented Mar 24, 2024

This PR did not account for query parameters so url like this content-manager/single-types/api::home-page.home-page/?locale=fr returns false, because last item is ?locale=fr. Breaking while saving non default locale.

@Marc-Roig
Copy link
Contributor Author

Marc-Roig commented Mar 25, 2024

@minht11 thank you for the report :) Will create a ticket with the fix today

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: fix This PR is fixing a bug source: plugin:i18n Source is plugin/i18n package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue directly publishing a translated entity after translating
5 participants