Skip to content

Support statusCode default param when loading template directly via route #41414

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

Merged
merged 1 commit into from
Oct 27, 2021

Conversation

dayallnash
Copy link
Contributor

@dayallnash dayallnash commented May 26, 2021

This is my first PR to Symfony, so please be patient as I get to grips with the 'admin' process of getting everything exactly how you want it!

Q A
Branch? 5.4
Bug fix? no
New feature? yes
Deprecations? no
Tickets
License MIT
Doc PR symfony/symfony-docs#15376

TODO

  • submit changes to the documentation and update this PR with a link

Summary

Added support for statusCode default parameter when loading a template directly from route via the Symfony\Bundle\FrameworkBundle\Controller\TemplateController controller (like this). This will continue to default to a 200 code, but can be changed by updating your route - for instance something like this:

test_route:
  path: /test_route
  controller:    Symfony\Bundle\FrameworkBundle\Controller\TemplateController
  defaults:
    # the path of the template to render
    template:  'test.html.twig'
    # the status code to include in the response headers
    statusCode: 201

This could be useful for when you want to render a template without adding any extra business logic in a controller, but don't want to return a 200 response.

dayallnash added a commit to dayallnash/symfony-docs that referenced this pull request May 26, 2021
Update docs to provide info on potential new feature found here: symfony/symfony#41414
@dayallnash dayallnash force-pushed the set_status_code_in_route branch from 2cfa7cb to b8e9f85 Compare May 26, 2021 11:34
@javiereguiluz
Copy link
Member

Dale, thanks for proposing this contribution! Don't worry about the actual contribution process because we have a lot of patience with newcomers. However, if you have any questions or comments during this process or if you don't feel treated well for any reasons, please tell us so we can quickly fix that. Thanks!

@dayallnash dayallnash changed the title Support statusCode default param when loading template directly via route [FrameworkBundle] Support statusCode default param when loading template directly via route May 26, 2021
@dayallnash
Copy link
Contributor Author

Thanks @javiereguiluz - I've got the PR in a state where it's passing all of the workflows now at least!

Copy link
Contributor

@YaFou YaFou left a comment

Choose a reason for hiding this comment

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

Promising start!

@carsonbot carsonbot changed the title [FrameworkBundle] Support statusCode default param when loading template directly via route Support statusCode default param when loading template directly via route May 27, 2021
@nicolas-grekas nicolas-grekas added this to the 5.4 milestone May 27, 2021
@dayallnash
Copy link
Contributor Author

@carsonbot find me a reviewer please

@carsonbot
Copy link

I'm sorry. I could not find any suitable reviewer.

@dayallnash
Copy link
Contributor Author

I'm sorry. I could not find any suitable reviewer.

It's ok, you tried. Good bot.

@GromNaN
Copy link
Member

GromNaN commented May 30, 2021

Nice feature. Useful to return a 410 when a page is removed.

Copy link
Member

@dunglas dunglas left a comment

Choose a reason for hiding this comment

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

LGTM, +1 when @derrabus' comment will be handled.

@dayallnash
Copy link
Contributor Author

Sorry about all the commits - I am still getting my head around rebasing (I normally do merging instead). PR should be in a good state now for final review.

@dayallnash dayallnash requested review from derrabus and dunglas June 3, 2021 12:41
@dayallnash dayallnash requested a review from OskarStark June 4, 2021 11:36
@dayallnash
Copy link
Contributor Author

The failed tests on TravisCI (PHP 7.2 build) seem unrelated to my change

@derrabus
Copy link
Member

derrabus commented Jun 9, 2021

There are a lot of commits in the PR that don't belong here. Can you sort that out?

@dayallnash
Copy link
Contributor Author

There are a lot of commits in the PR that don't belong here. Can you sort that out?

I am trying - I just still don't understand rebasing yet. Every time I do it I am prompted to fix loads of conflicts and end up with loads of extra commits on the PR 😞

@dayallnash dayallnash force-pushed the set_status_code_in_route branch from 864dc4d to c49cf7f Compare June 9, 2021 11:24
@fabpot fabpot force-pushed the set_status_code_in_route branch 2 times, most recently from 318f0e1 to 521fe37 Compare October 27, 2021 17:44
@fabpot
Copy link
Member

fabpot commented Oct 27, 2021

To help finish this PR, I've squashed the commits, and rebased on the current 5.4 branch.

…ate directly from route using the `Symfony\Bundle\FrameworkBundle\Controller\TemplateController` controller.
@fabpot fabpot force-pushed the set_status_code_in_route branch from 521fe37 to 5a7b666 Compare October 27, 2021 17:52
@fabpot
Copy link
Member

fabpot commented Oct 27, 2021

Thank you @dayallnash.

@fabpot fabpot merged commit ee6bbb2 into symfony:5.4 Oct 27, 2021
@dayallnash dayallnash deleted the set_status_code_in_route branch October 29, 2021 09:13
This was referenced Nov 5, 2021
javiereguiluz added a commit to javiereguiluz/symfony-docs that referenced this pull request Dec 2, 2021
…(dayallnash)

This PR was merged into the 5.4 branch.

Discussion
----------

Update templates.rst for PR symfony/symfony#41414

Update docs to provide info on potential new feature found here: symfony/symfony#41414

Commits
-------

3acf948 Update templates.rst
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.

10 participants