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

SANTA-222: symfony upgrade #581

Merged
merged 21 commits into from
Oct 14, 2024
Merged

Conversation

ChuckCCW
Copy link

No description provided.

@tvlooy
Copy link
Collaborator

tvlooy commented Aug 29, 2024

@ChuckCCW the tests fail, you can have a look and clear any errors until they are green. I think if you push a new test run starts, if not let me know!

@tvlooy
Copy link
Collaborator

tvlooy commented Sep 1, 2024

@ChuckCCW is the MR is ready to go?

$mailer: '@swiftmailer.mailer.standard_mailer'
$mandrill: '@swiftmailer.mailer.mandrill_mailer'
$mailer: '@mailer'
$mandrill: '@mailer'
Copy link
Collaborator

Choose a reason for hiding this comment

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

dit zijn 2 aparte mailers, eentje via localhost (standard mailer) en eentje via mandrill. De code kiest op basis van het domein welke mailer er gebruikt wordt https://github.com/iodigital-com/SecretSanta/blob/master/src/Mailer/MailerService.php#L571 Maar deze change zet dus alles op de default mailer

defaults:
_controller: Symfony\Bundle\FrameworkBundle\Controller\TemplateController::templateAction
template: 'Static/cookiePolicy.html.twig'

ecofriendly1:
path: /eco-friendly-tips
path: /{_locale}/eco-friendly-tips
Copy link
Collaborator

Choose a reason for hiding this comment

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

de URL's zonder locale werken niet meer, dat zijn net de URL's die in google zitten volgens mij. De pagina zonder locale zou op de EN versie moeten uitkomen

protected function configure()
{
protected function configure(): void
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

CS

protected function configure()
{
protected function configure(): void
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

CS

public function editParticipantAction(Request $request, string $listurl, Participant $participant, ParticipantService $participantService, MailerService $mailerService)
{
/**
* @throws TransportExceptionInterface
Copy link
Collaborator

Choose a reason for hiding this comment

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

Dit is een HTTP endpoint, die zouden geen exceptions meer mogen throwen. Of het voelt althans niet correct om het hie zo te documenteren.

$em->persist($assignedParticipant);
$em->flush();
if ($assignedParticipant) {
$assignedParticipant->setAssignedParticipant($secretSanta);
Copy link
Collaborator

Choose a reason for hiding this comment

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

CS

* @Route("/participant/{url}", name="participant_view", methods={"GET"})
* @Template("Participant/show/valid.html.twig")
*/
#[Route("/{_locale}/participant/{url}", name: "participant_view", methods: ["GET"])]
Copy link
Collaborator

Choose a reason for hiding this comment

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

misschien niet slecht om friendsofphp/php-cs-fixer in --dev te includen en PHP_CS_FIXER_FUTURE_MODE=1 php ./vendor/bin/php-cs-fixer fix -v --dry-run --diff toe te voegen om die indents te fixen

@@ -4,11 +4,12 @@

class BlacklistEmail
{
// @phpstan-ignore-next-line
Copy link
Collaborator

@tvlooy tvlooy Sep 2, 2024

Choose a reason for hiding this comment

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

hiervoor kan je phpstan/phpstan-doctrine toevoegen. Dan in phpstan.neon:

includes:
    - vendor/phpstan/phpstan-doctrine/extension.neon
parameters:
    doctrine:
        objectManagerLoader: tests/object-manager.php

Ik kan je een object-mapper..php file doorsturen.

@tvlooy tvlooy merged commit 8d3d367 into iodigital-com:master Oct 14, 2024
1 check passed
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