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

[5.x] Fix taxonomy routes on multi-site #10398

Merged
merged 6 commits into from
Jul 19, 2024

Conversation

aerni
Copy link
Contributor

@aerni aerni commented Jul 4, 2024

This PR fixes an issue with taxonomy routes on multi-sites. Previously, the taxonomy was accessible on all sites, even on sites that haven't been configured on the taxonomy.

Consider these configs.

resources/sites.yaml

english:
  name: English
  url: /
  locale: en_US
german:
  name: German
  url: /de
  locale: de_DE
french:
  name: French
  url: /fr
  locale: fr_FR

content/taxonomies/tags.yaml

title: Tags
sites:
  - english
  - german

If you've created the necessary views to get the taxonomy routes working, the taxonomy would be accessible on:

  • /tags
  • /de/tags
  • /fr/tags

But the taxonomy was only configured for the english and german sites. This PR fixes this issue. So now the taxonomy is only accessible on:

  • /tags
  • /de/tags

This fix also applies to collection taxonomy routes. Here, however, the collection's sites will also be considered. In this case, both the taxonomy and the collection need to be configured for the english and german sites.

  • /pages/tags
  • /de/pages/tags

@aerni aerni changed the title Fix taxonomy routes on multi-site [5.x] Fix taxonomy routes on multi-site Jul 4, 2024
@aerni
Copy link
Contributor Author

aerni commented Jul 17, 2024

Hmm, thinking about this a little more, the collection taxonomy routes should probably only be accessible on sites that have been configured on both the collection and the taxonomy. As it stands with this PR, they are accessible for all sites that are configured on the taxonomy. But let's say you have this route /de/pages/tags, you likely want it to throw a 404 if the pages collection or the tags taxonomy isn't configured for the german site.

@jasonvarga
Copy link
Member

Yes that sounds right. 👍

@jasonvarga jasonvarga merged commit 10beda2 into statamic:5.x Jul 19, 2024
16 checks passed
{
$this->mountBlogPageToBlogCollection();

// Set all the slugs to match the taxonomy, to make sure that the
Copy link
Member

Choose a reason for hiding this comment

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

I think I meant "Set all the slugs to match the taxonomy collection"

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.

2 participants