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 displaying (singleton) page by slug #1921

Merged
merged 1 commit into from
Sep 30, 2020

Conversation

denis-gorin
Copy link
Contributor

Fixes #1920

if we allow numeric slug
I suppose to convert numeric slug to String couse not to brake repository method findOneBySlug

@bobdenotter
Copy link
Member

Hi @denis-gorin,

How are you triggering this? Are you calling the method yourself, instead of via routing? AFAIK, it should always be a string already, even if it's a numeric one.

@denis-gorin
Copy link
Contributor Author

Hi @denis-gorin,

How are you triggering this? Are you calling the method yourself, instead of via routing? AFAIK, it should always be a string already, even if it's a numeric one.

  • There is nothing in my config\routes.yaml
  • I have several singletones
  • and corresonding to singletone's names - several templates (ex. company => company.twig)

when I call /company page I've got an error

Argument 1 passed to Bolt\Repository\ContentRepository::findOneBySlug() must be of the type string, integer given

ContentRepository->findOneBySlug(2, object(ContentType))

called in \vendor\bolt\core\src\Controller\Frontend\DetailController.php on line 46

Request info

Matched route "listing_locale".

{
    "route": "listing_locale",
    "route_parameters": {
        "_route": "listing_locale",
        "_controller": "Bolt\\Controller\\Frontend\\ListingController::listing",
        "_locale": "en",
        "contentTypeSlug": "company"
    },
    "request_uri": "https://127.0.0.1:8000/en/company",
    "method": "GET"
}

@develth
Copy link
Contributor

develth commented Sep 29, 2020

Confirm, have the same error. This PR fixes it

@I-Valchev
Copy link
Member

@denis-gorin could it be that the param converter is trying to be smart and type hints the slug? If that's so, maybe we can just do like so:

public function findOneBySlug(string $slug, ?ContentType $contentType = null): ?Content

rather than casting it to a string?

@denis-gorin
Copy link
Contributor Author

public function findOneBySlug(string $slug, ?ContentType $contentType = null): ?Content

exactly that it was
so, method findOneBySlug waiting for a string but got an integer

The roots of an integer are in this

//src: src\Controller\Frontend\ListingController.php:61

return $this->forward($controller, ['slugOrId' => $content->getId()]);

... trying to "list" a singleton, we're getting a single Content by Id

@bobdenotter
Copy link
Member

@denis-gorin I've managed to reproduce it! Thanks for the extra info! We'll merge in, after Travis passes.

@I-Valchev It's already typehinted as string. I believe conversion like that is only done in Autowiring (in calling PHP code), or by PHP but then only in absence of strict_types=1.

@bobdenotter
Copy link
Member

For those curious: I've looked it up, and this indeed got broken very recently. I introduced this bug here: #1884

Copy link
Member

@I-Valchev I-Valchev left a comment

Choose a reason for hiding this comment

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

👍 Thanks for fixing this @denis-gorin . I think we're ready to roll with this PR once the final test passes :-)

@bobdenotter bobdenotter merged commit 918da75 into bolt:master Sep 30, 2020
@bobdenotter bobdenotter changed the title fix displaying page (singleton) by slug Fix displaying (singleton) page by slug Sep 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

error displaying page (singleton) by slug
4 participants