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

Return value of Kirby\Cms\Model::__toString() is null when called from $site #2802

Closed
tritos-design opened this issue Sep 3, 2020 · 4 comments · Fixed by #2808
Closed

Return value of Kirby\Cms\Model::__toString() is null when called from $site #2802

tritos-design opened this issue Sep 3, 2020 · 4 comments · Fixed by #2808
Labels
type: bug 🐛 Is a bug; fixes a bug
Milestone

Comments

@tritos-design
Copy link

Describe the bug
Return value of Kirby\Cms\Model::__toString() is null when called from $site, e.g. by echo $site.

To Reproduce
Add echo $site to any template php file. This produces to above mentioned PHP error.

Expected behavior
__toString() should either return the empty string or throw an exception.

Kirby Version
3.4.2

@afbora afbora added the type: bug 🐛 Is a bug; fixes a bug label Sep 3, 2020
@afbora
Copy link
Member

afbora commented Sep 3, 2020

Because Site has no id and Site::id() method doesn't exists. Calling as default Model::id() (parent class) method that returns null.

@bastianallgeier
Copy link
Member

The biggest question here is, what would be a useful value to return in case of the Site object?

@afbora
Copy link
Member

afbora commented Sep 4, 2020

Would that be too bad for Site model? 🙈

public function id(): string
{
    return '';
}

@bastianallgeier
Copy link
Member

I didn't want to add a new id method that just does nothing useful at all. So I only overwrote the __toString method in the site class and returned the URL there.

@bastianallgeier bastianallgeier added this to the 3.4.3 milestone Sep 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug 🐛 Is a bug; fixes a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants