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

[4.x] Use default slug to get taxonomy entries count #8743

Merged
merged 5 commits into from Sep 20, 2023

Conversation

ryanmitchell
Copy link
Contributor

This may be too simplistic a fix, but to get the correct entry count raised in #4493 we could just make sure we query by the default locale.

Fixes #4493

@ryanmitchell ryanmitchell changed the title Use default slug to get taxonomy entries count [4.x] Use default slug to get taxonomy entries count Sep 20, 2023
@ryanmitchell
Copy link
Contributor Author

Yeah same test failures as on 5686. Something is definitely broken upstream.

@jasonvarga
Copy link
Member

I was just working on this too and noticed we already have test coverage for this. But looks like the tests aren't set up right somehow since it passes before this change too.

My fix was the same as yours though 👍 Just figuring out what's wrong with the tests. The slugs in the localized versions aren't being used.

@ryanmitchell
Copy link
Contributor Author

Cool. Want to close this in favour of yours?

@jasonvarga
Copy link
Member

Nope leave this, I'll push to yours.

I'll sort out the unrelated test failures too.

@ryanmitchell
Copy link
Contributor Author

Seems like the tests aren't ever checking for a count on a localised slug? Only ever a slug in the primary language.

@jasonvarga
Copy link
Member

Yeah that's reason people are reporting #4493, and your PR fixes that.

But the tests were passing because the slug it looks like we're setting on the localizations don't ever actually make it to the term. If the localized term doesn't have a different slug, the bug never happens. No worries, I'll sort it out. Thanks!

@jasonvarga
Copy link
Member

The unrelated test failures fixed over here: #8746

@jasonvarga jasonvarga merged commit b97db8e into statamic:4.x Sep 20, 2023
16 checks passed
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

Successfully merging this pull request may close these issues.

entries_count is zero in multisite?
2 participants