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] Respect the current site when returning a View #10109

Merged
merged 1 commit into from
May 17, 2024

Conversation

aerni
Copy link
Contributor

@aerni aerni commented May 17, 2024

This PR fixes an issue where the current site wasn't resolved correctly when returning a view from an action route. Consider the following scenario:

Example

I'm returning a View in a controller from an action route.

return (new View)
    ->layout($layout)
    ->template($template)
    ->cascadeContent($data);

When the cascade() is hydrated in the View class, it will resolve the Cascade::instance()from the container, where it was bound with Site::current().

cms/src/View/View.php

Lines 188 to 194 in 5cb5da6

protected function cascade()
{
return $this->cascade = $this->cascade ?? Cascade::instance()
->withContent($this->cascadeContent)
->hydrate()
->toArray();
}

$this->app->singleton(Cascade::class, function ($app) {
return new Cascade($app['request'], Site::current());
});

At the time when the Cascade is bound in the container, the current site will be resolved from the current URL. Because we are on an action route, the resolved site will always be the default site.

cms/src/Sites/Sites.php

Lines 72 to 77 in 5cb5da6

private function findByCurrentUrl()
{
return $this->findByUrl(
$this->currentUrlCallback ? call_user_func($this->currentUrlCallback) : request()->getUri()
);
}

This leads to the View always being hydrated with data from the default site.

In my scenario, I rely on localized content being returned from the action route. To solve this, I figured that I could set the current site before returning the view:

Site::setCurrent($site);

return (new View)
    ->layout($layout)
    ->template($template)
    ->cascadeContent($data);

However, this won't work, as the Cascade was already bound with the site resolved from the current request before Site::setCurrent($site) is called.

To resolve this issue, we can explicitly set the current site using the withSite() method before the Cascade is hydrated in the cascade() method of the View class.

aerni added a commit to aerni/statamic-advanced-seo that referenced this pull request May 17, 2024
@jasonvarga jasonvarga merged commit 61f487c into statamic:5.x May 17, 2024
17 checks passed
@jasonvarga
Copy link
Member

Beautiful PR, thanks.

@aerni aerni deleted the fix/view-current-site branch May 17, 2024 20:08
aerni added a commit to aerni/statamic-advanced-seo that referenced this pull request May 20, 2024
aerni added a commit to aerni/statamic-advanced-seo that referenced this pull request May 20, 2024
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