Skip to content

Conversation

@aurelijusb
Copy link
Owner

@aurelijusb aurelijusb commented Dec 3, 2019

Rebased from #91
To include latest tests

* @return Response
*/
public function index()
public function index(KernelInterface $request)
Copy link
Owner Author

Choose a reason for hiding this comment

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

👍 Šaunu, kad naudoji KernelInterface – tada ramiau galima judinti PHP failus tarp skirtingų katalogų

{% for key, team in data %}
{% for student in team.students %}
<li class="list-group-item">
<a href="{{ path('student', {name: student, project: key})|escape }}">{{ student }}</a>
Copy link
Owner Author

@aurelijusb aurelijusb Dec 3, 2019

Choose a reason for hiding this comment

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

Symfony'je Twig'as standartinškai yra įjungęs autoescape

Suggested change
<a href="{{ path('student', {name: student, project: key})|escape }}">{{ student }}</a>
<a href="{{ path('student', {name: student, project: key}) }}">{{ student }}</a>

</ul>

<div class="m-4">
<form action="/student" method="get">
Copy link
Owner Author

Choose a reason for hiding this comment

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

Reikėtų įprasti visur naudoti path. Nes ateityje bus sunku visus hard-coded variantus sugaudyti

@aurelijusb aurelijusb marked this pull request as ready for review December 3, 2019 19:33
@aurelijusb
Copy link
Owner Author

Esmė gera,
❌ bet jei pataisytum smulkias dalis dėl path ir escape – būtų idealu

Pasitikrinimui yra testai: https://github.com/aurelijusb/kickstart/pull/288/files:
image

@aurelijusb aurelijusb mentioned this pull request Dec 3, 2019
@aurelijusb
Copy link
Owner Author

Beje. Dėkui už tvarkingai atliktą darbą ir informavimą apie potencialias klaidas (51 studentą)

@aurelijusb
Copy link
Owner Author

aurelijusb commented Dec 3, 2019

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.

3 participants